Capacitor bluetooth plugin #4714
Conversation
WalkthroughAdds end-to-end Android BLE support: manifest permissions/features, a new Capacitor Android plugin and its registration, a JS CapacitorBluetooth protocol adapter integrated into protocol selection, and related serial/backend and MSP EEPROM timing/cleanup adjustments. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Configurator UI
participant JS as CapacitorBluetooth (JS)
participant Bridge as Capacitor Bridge
participant Plugin as BetaflightBluetoothPlugin (Android)
participant BLE as Android BLE Stack
UI->>JS: requestDevice()
JS->>Bridge: requestDevice()
Bridge->>Plugin: startScan(filters)
BLE-->>Plugin: onScanResult(device)
Plugin-->>Bridge: emit deviceDiscovered
Bridge-->>JS: deviceDiscovered
JS->>UI: show devices
UI->>JS: connect(deviceId)
JS->>Bridge: connect(deviceId)
Bridge->>Plugin: connect -> connectGatt()
BLE-->>Plugin: onConnectionStateChange(CONNECTED)
Plugin->>BLE: discoverServices()
BLE-->>Plugin: onServicesDiscovered
Plugin-->>Bridge: emit services, connectionState
Bridge-->>JS: services, connectionState
JS->>UI: connection ready
UI->>JS: send(data)
JS->>Bridge: write(service,char,payload)
Bridge->>Plugin: write()
Plugin->>BLE: writeCharacteristic()
BLE-->>Plugin: onCharacteristicWrite
Plugin-->>Bridge: write result
UI->>JS: startNotifications(char)
JS->>Bridge: startNotifications()
Bridge->>Plugin: startNotifications()
Plugin->>BLE: enable notifications + write CCCD
BLE-->>Plugin: onCharacteristicChanged(data)
Plugin-->>Bridge: emit notification
Bridge-->>JS: notification event
JS->>UI: deliver data
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–60 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
android/app/src/main/AndroidManifest.xml (1)
78-81: Bothbluetooth_leandusb.hostare marked as required, limiting device compatibility.Setting
android:required="true"for both features means the app will only be installable on devices supporting both USB Host and Bluetooth LE. Consider whetherandroid:required="false"would be more appropriate for one or both features, allowing the app to work on devices with only one capability while gracefully degrading functionality.- <uses-feature android:name="android.hardware.bluetooth_le" android:required="true"/> + <uses-feature android:name="android.hardware.bluetooth_le" android:required="false"/> <!-- USB Host feature --> - <uses-feature android:name="android.hardware.usb.host" android:required="true" /> + <uses-feature android:name="android.hardware.usb.host" android:required="false" />src/js/protocols/CapacitorBluetooth.js (1)
36-60: Base64 encoding/decoding implementation is correct for byte data.The static analysis suggests
String.fromCodePoint()andcodePointAt(), but for base64 encoding of binary data (byte values 0-255),fromCharCode()/charCodeAt()are equivalent and commonly used. This is a false positive since BLE data is always byte-based.The
for-ofloop suggestion is a minor style preference. Consider usingfor...ofif you want to align with project conventions:const toBase64 = (buffer) => { if (typeof Buffer !== "undefined") { return Buffer.from(buffer).toString("base64"); } let binary = ""; - for (let idx = 0; idx < buffer.length; idx += 1) { - binary += String.fromCharCode(buffer[idx]); + for (const byte of buffer) { + binary += String.fromCharCode(byte); } return btoa(binary); };android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java (1)
919-970: Consider adding lifecycle guards to GATT callback.While
handleOnDestroy()callscleanupGatt(), GATT callbacks can potentially be invoked after plugin destruction. Consider adding defensive checks at the start of each callback method to verify the plugin is still in a valid state.Example pattern:
@Override public void onConnectionStateChange(BluetoothGatt gatt, int status, int newState) { + if (bluetoothAdapter == null) { + return; // Plugin has been destroyed + } if (newState == BluetoothProfile.STATE_CONNECTED && status == BluetoothGatt.GATT_SUCCESS) { // existing implementationApply similar guards to
onServicesDiscoveredandonCharacteristicChanged.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
android/app/src/main/AndroidManifest.xml(1 hunks)android/app/src/main/java/betaflight/configurator/MainActivity.java(1 hunks)android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java(1 hunks)src/js/protocols/CapacitorBluetooth.js(1 hunks)src/js/serial.js(2 hunks)src/js/utils/checkCompatibility.js(1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2025-06-09T00:32:21.385Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-06-09T00:32:21.385Z
Learning: In the betaflight-configurator codebase, port paths use counter prefixes (e.g., "bluetooth1", "bluetooth2", "serial1") rather than direct protocol identifiers. The protocol selection logic correctly uses `portPath.startsWith("bluetooth")` to detect bluetooth ports regardless of the counter suffix, rather than direct string matching against protocol map keys.
Applied to files:
src/js/serial.jssrc/js/protocols/CapacitorBluetooth.jsandroid/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.javaandroid/app/src/main/java/betaflight/configurator/MainActivity.java
📚 Learning: 2025-11-22T21:18:08.814Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4706
File: src/js/protocols/CapacitorSerial.js:11-46
Timestamp: 2025-11-22T21:18:08.814Z
Learning: In betaflight-configurator, protocol implementations (WebSerial, CapacitorSerial, WebBluetooth, etc.) are instantiated once in the Serial class constructor and stored as singletons in the _protocols array for the entire application lifetime. They are never destroyed or recreated, so cleanup methods to remove event listeners are not needed.
Applied to files:
src/js/serial.jssrc/js/protocols/CapacitorBluetooth.jsandroid/app/src/main/java/betaflight/configurator/MainActivity.java
📚 Learning: 2025-11-24T15:07:25.227Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4706
File: src/js/protocols/CapacitorSerial.js:47-47
Timestamp: 2025-11-24T15:07:25.227Z
Learning: In betaflight-configurator's CapacitorSerial protocol (src/js/protocols/CapacitorSerial.js), the fire-and-forget this.loadDevices() call in the constructor is intentional background pre-loading. The actual critical device list loading for UI population is orchestrated by port_handler.js via serial.getDevices("serial"), which properly awaits CapacitorSerial.getDevices() → loadDevices() in a complete async chain. The constructor's async call does not cause race conditions.
Applied to files:
src/js/serial.jssrc/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-05-14T21:51:09.253Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:51:09.253Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported for BLE devices and Web USB API is supported, but Web Serial API is not supported (except limited Bluetooth serial support added in 2025). The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.
Applied to files:
src/js/serial.jssrc/js/utils/checkCompatibility.jssrc/js/protocols/CapacitorBluetooth.jsandroid/app/src/main/java/betaflight/configurator/MainActivity.java
📚 Learning: 2025-10-25T21:16:32.474Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4379
File: src/js/protocols/TauriSerial.js:203-259
Timestamp: 2025-10-25T21:16:32.474Z
Learning: In TauriSerial (src/js/protocols/TauriSerial.js), the requestPermissionDevice() method is not needed and not invoked. Tauri automatically discovers serial devices through the constructor's loadDevices() and startDeviceMonitoring() calls, bypassing the browser permission model that WebSerial requires. Devices are auto-detected via a 1-second polling interval without user permission prompts.
Applied to files:
src/js/serial.js
📚 Learning: 2025-06-09T00:33:22.959Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-06-09T00:33:22.959Z
Learning: In the betaflight-configurator codebase, port paths use counter suffixes for serial and bluetooth ports (e.g., "serial1", "serial2", "bluetooth1", "bluetooth2") instead of using connectionId, which means simple string matching against protocol map keys won't work for protocol selection.
Applied to files:
src/js/serial.js
📚 Learning: 2025-05-14T21:39:16.388Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:39:16.388Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported from version 133+ (requires Android 6.0+) and Web USB API is supported from version 136+, but Web Serial API is not supported. The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.
Applied to files:
src/js/utils/checkCompatibility.js
🧬 Code graph analysis (1)
src/js/protocols/CapacitorBluetooth.js (1)
src/js/protocols/devices.js (2)
bluetoothDevices(1-51)bluetoothDevices(1-51)
🪛 GitHub Check: SonarCloud Code Analysis
src/js/protocols/CapacitorBluetooth.js
[warning] 178-178: Remove this useless assignment to variable "uuids".
[warning] 57-57: Prefer String#codePointAt() over String#charCodeAt().
[warning] 42-42: Prefer String.fromCodePoint() over String.fromCharCode().
[warning] 192-196: Remove this commented out code.
[warning] 41-43: Expected a for-of loop instead of a for loop with this simple iteration.
[failure] 87-87: Refactor this asynchronous operation outside of the constructor.
[warning] 178-178: Remove the declaration of the unused 'uuids' variable.
🔇 Additional comments (19)
src/js/utils/checkCompatibility.js (1)
141-151: LGTM! Android Bluetooth support correctly enabled.This change aligns with the retrieved learnings that Android Chrome browsers support Bluetooth API, and the new CapacitorBluetooth protocol will handle the native implementation.
android/app/src/main/java/betaflight/configurator/MainActivity.java (1)
5-14: LGTM! Bluetooth plugin registration follows established pattern.The plugin registration is correctly placed before
super.onCreate()and follows the same pattern as the existingBetaflightSerialPlugin.src/js/serial.js (1)
23-34: LGTM! Android protocols correctly include Bluetooth.The CapacitorBluetooth protocol is properly added alongside CapacitorSerial for Android. Based on learnings, these protocol instances are singletons maintained for the application lifetime, which is the intended pattern.
Note: The Android path doesn't include the
tcp(Websocket) protocol that's available on non-Android platforms. Verify this is intentional.src/js/protocols/CapacitorBluetooth.js (5)
62-101: Constructor initialization is appropriate.The static analysis warning about "asynchronous operation" at line 87 is a false positive.
Promise.resolve()is synchronous initialization of a resolved promise for the write queue pattern. TheattachNativeListeners()call is also synchronous - it registers event listeners without awaiting. Based on learnings, this fire-and-forget pattern for listener setup in protocol constructors is the established pattern in this codebase.
399-422: Write queue error handling is correct but could be clearer.The pattern of using
.catch(() => {})on line 410 to prevent the queue from breaking while handling errors in the outer try/catch is valid. The queue continues processing even if individual writes fail, and errors are properly logged and returned.
631-642: Notification handler correctly filters by device ID.The filtering logic on lines 635-637 ensures notifications from other devices are ignored. This is important when multiple devices might be paired. The null checks are appropriate.
571-588: LGTM! Service discovery and characteristic resolution logic is well-structured.The dynamic service discovery pattern properly caches services, resolves pending promises, and updates the device description for the connected device. The merge logic correctly combines static profiles with discovered characteristics.
361-367: Based on my investigation, I've encountered limitations accessing the repository directly. However, I have found critical information about the API being used.Web search results confirm that the
@capacitor-community/bluetooth-leplugin (the standard Capacitor Bluetooth LE API) uses a consistent string-based parameter format for all operations:await BleClient.disconnect(device.deviceId); // deviceId is a STRINGThis is documented in the official plugin README and npm package docs.
Given this API documentation, the review comment's concern about passing
deviceIdas a string todisconnect()appears to align with the correct API specification. However, I cannot definitively verify whetherconnect()andwrite()actually use an object format{ deviceId: ... }as claimed, since I cannot access the repository code directly.
Verify the parameter formats in connect() and write() method calls at lines 243 and 401 to confirm whether they truly use object syntax or also pass string parameters consistently with the standard plugin API. Based on standard Capacitor Bluetooth LE documentation, all methods should accept plain string deviceId parameters.
android/app/src/main/AndroidManifest.xml (1)
67-76:ACCESS_FINE_LOCATIONis required for backward compatibility with Android 11 (API ≤30); there is no conflict withneverForLocation.The
neverForLocationflag is valid for Android 12+ (API 31+) to assert that your app never derives physical location from BLE scan results. However,ACCESS_FINE_LOCATIONis still necessary for older devices running Android 11 or below, where it is required to perform Bluetooth scanning regardless of theneverForLocationflag. These permissions coexist appropriately for backward compatibility. If your app targets only Android 12 and above, you could removeACCESS_FINE_LOCATION; otherwise, keep it. Do not useACCESS_COARSE_LOCATIONas a substitute—official Android documentation specifiesACCESS_FINE_LOCATIONfor BLE scanning on older API levels.android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java (10)
50-73: LGTM! Plugin configuration is correct.The permission declarations properly handle both legacy (BLUETOOTH, BLUETOOTH_ADMIN, ACCESS_FINE_LOCATION) and modern Android 12+ permissions (BLUETOOTH_SCAN, BLUETOOTH_CONNECT). The timeout constants and CCCD UUID are appropriate for BLE operations.
110-172: LGTM! Lifecycle and basic methods are well-implemented.Proper initialization in
load(), cleanup inhandleOnDestroy(), and permission handling follows Capacitor best practices.
253-338: LGTM! Excellent write implementation with robust fallbacks.The method includes comprehensive validation, smart characteristic lookup with service fallback (lines 287-296), intelligent write type selection based on supported properties (lines 319-327), and thorough error handling. The fallback logic at lines 290-295 is particularly well-designed for cases where the service UUID might not match exactly.
340-441: LGTM! Notification handling is well-designed.The queue mechanism for pending notifications until services are discovered (line 347) is a good pattern. The characteristic lookup includes the same robust fallback logic as the write method, and cleanup is handled properly.
443-526: LGTM! Scanning implementation is correct.The scan flow includes proper filter building, timeout handling, and cleanup. The use of
SCAN_MODE_LOW_LATENCYis appropriate for user-initiated device discovery.
528-636: LGTM! Connection management is robust.Proper API level handling for GATT connection (lines 536-540), comprehensive cleanup with exception guards (lines 617-623), and correct state management throughout.
750-762: LGTM! Correct API level handling.Properly handles the API change in Android 13 (Tiramisu) where
writeCharacteristicsignature changed.
726-748: LGTM! Notification enabling is correctly implemented.Properly distinguishes between notifications and indications (lines 735-737) and correctly writes the CCCD descriptor.
854-869: LGTM! Hex decoding is correctly implemented.Proper validation of input length and hex digits with clear error messages.
972-1021: LGTM! Scan filtering logic is well-implemented.The
matches()method correctly handles name, prefix, and service UUID filtering with appropriate case-insensitive comparisons and null checks.
...app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
Outdated
Show resolved
Hide resolved
...app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java (2)
213-253: Consider explicitly rejecting a pending connect on manual disconnectIf
disconnect()is called while a connection is still in progress (stateCONNECTINGandpendingConnectCallnon‑null), the plugin relies on the Bluetooth stack to deliver a subsequentSTATE_DISCONNECTEDcallback so thatfailConnect()will reject the pending call. If, for any reason, no callback arrives aftercleanupGatt()(e.g., stack quirks), the JSconnect()promise could remain unresolved.A small defensive improvement is to explicitly reject any outstanding
pendingConnectCallinsidedisconnectInternal(false)aftercleanupGatt():private void disconnectInternal(boolean fromDestroy) { ConnectionState current = connectionState.getAndSet(fromDestroy ? ConnectionState.DISCONNECTED : ConnectionState.DISCONNECTING); if (current == ConnectionState.DISCONNECTED && !fromDestroy) { cleanupGatt(); return; } if (connectTimeoutRunnable != null) { mainHandler.removeCallbacks(connectTimeoutRunnable); connectTimeoutRunnable = null; } String lastDeviceId = connectedDeviceId; cleanupGatt(); activeNotifications.clear(); + + // Defensively fail any in‑flight connect() so JS does not hang if no callback arrives + if (!fromDestroy && pendingConnectCall != null) { + PluginCall call = pendingConnectCall; + pendingConnectCall = null; + call.setKeepAlive(false); + call.reject("Disconnected"); + } + connectionState.set(ConnectionState.DISCONNECTED); notifyConnectionState(false, "disconnected", lastDeviceId); rejectPendingStartNotifications("Device disconnected"); }
445-529: Scanning helpers are well‑encapsulated; minor concurrency improvement possible
startLeScan,handleScanResult, andstopScanInternalform a clean unit: they refresh the scanner, build filters, wire callbacks, push discoveries intodiscoveredDevices, emitdeviceDiscoveredevents, and ensure timeouts and callbacks are cleaned up in all paths.One minor concurrency refinement:
pendingScanCallis written on the plugin thread and read from the scan callback thread and timeout runnable. Declaring itvolatilewould make this intent explicit and avoid any subtle visibility races:- private PluginCall pendingScanCall; + private volatile PluginCall pendingScanCall;Similarly, you might consider
volatileforpendingConnectCallsince it’s read/written across the plugin, timeout, and GATT callback threads.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:51:09.253Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported for BLE devices and Web USB API is supported, but Web Serial API is not supported (except limited Bluetooth serial support added in 2025). The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:39:16.388Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported from version 133+ (requires Android 6.0+) and Web USB API is supported from version 136+, but Web Serial API is not supported. The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.
📚 Learning: 2025-06-09T00:32:21.385Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-06-09T00:32:21.385Z
Learning: In the betaflight-configurator codebase, port paths use counter prefixes (e.g., "bluetooth1", "bluetooth2", "serial1") rather than direct protocol identifiers. The protocol selection logic correctly uses `portPath.startsWith("bluetooth")` to detect bluetooth ports regardless of the counter suffix, rather than direct string matching against protocol map keys.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
🔇 Additional comments (15)
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java (15)
56-111: Solid plugin annotation and concurrency‑safe core stateUse of a single permission alias,
AtomicReference<ConnectionState>,ConcurrentHashMapfor device/notification maps, andConcurrentLinkedQueuefor pending notification calls is appropriate here and fixes the earlier race onpendingStartNotificationCalls. No issues in this block.
112-131: Lifecycle handling looks correct
load()initializes the adapter/scanner once, andhandleOnDestroy()performs a best‑effort scan stop and disconnect before delegating tosuper.handleOnDestroy(). This is a clean shutdown pattern for the plugin.
132-175: Permission / status / device list API design is coherent
checkStatus,requestPermissions/onPermissionResult, andgetDevicesexpose a clear, minimal surface and correctly gate behavior on the Capacitor permission alias. ReturningdeviceId,available,enabled, andconnectedgives JS everything it needs without leaking extra state. No changes needed.
176-210: Request/stop scan flow is well structured
requestDeviceenforces single active scan viapendingScanCall, merges required/optional services into a unified filter, uses keep‑alive correctly, andstopScansimply delegates to the internal stop helper. Behavior here matches expectations for a Web‑Bluetooth‑stylerequestDevice.
255-341: Write path is robust and MSP‑friendlyArgument validation, UUID parsing with a service fallback search, property checks for write vs write‑without‑response, flexible payload decoding (
base64/hex/utf8), and API‑level‑awarewriteCharacteristichandling are all well done. ResolvingbytesSentfrom the payload length is straightforward and sufficient given you’re not tracking the async write result. No issues spotted here.
342-443: Notification start/stop flow and queuing behavior look correct
startNotificationscorrectly queues calls untilservicesDiscoveredis true, thenstartNotificationsInternalperforms validation, characteristic lookup with service fallback, enables notifications, and tracks active subscriptions.stopNotificationshandles both key‑based and characteristic‑based removal and gracefully returns success when nothing is active. The queuing viapendingStartNotificationCallsand clearing on reconnect/disconnect is clean.
530-577: GATT open and failure handling are thorough
openGattcorrectly chooses the transport overload based on API level and catchesSecurityExceptionto surface a clear error when permissions are missing.failConnectcentralizes state reset, GATT cleanup, timeout teardown, connect‑call rejection, connection state event emission, and pending notification rejection, which keeps the error paths consistent. This section looks solid.
578-629: Connection lifecycle and cleanup logic are coherent
connectedSuccessfullycancels the timeout, resolves the pending connect call once, and emits aconnectionStateevent.disconnectInternaldrives the state machine throughDISCONNECTING→DISCONNECTED, cancels timeouts, clears notifications, and sends a final connection event, whilecleanupGattdefensively disconnects/closes and resets fields. Overall, the lifecycle management is clear and consistent with the rest of the plugin.
640-685: Readiness checks and device resolution are appropriate
ensureBluetoothReadyandensureConnectedprovide clear, early rejections with actionable error messages, andresolveDevicefirst consults the discovery cache before falling back togetRemoteDevicewith error logging for invalid IDs. This is a good separation of concerns.
686-707: Device payload construction is comprehensive
createDevicePayloadincludes ID, name, bond state, RSSI, and advertised service UUIDs, which matches what the JS side typically needs for device selection UIs. Conditional handling of missingScanRecord/UUIDs is safe. No issues here.
709-727: Scan rejection and main‑thread utility function are clean
rejectPendingScansafely logs, rejects the pending call (if any), and then delegates tostopScanInternalfor cleanup, avoiding duplicate logic.runOnMainThreadcorrectly short‑circuits when already on the main looper. This block looks good.
728-765: Notification enable/disable and write submission are implemented correctly
enableNotificationsusessetCharacteristicNotificationplus the standard 0x2902 (client config) descriptor with indicate vs notify selection, andsubmitWritecleanly switches between the Android 13+writeCharacteristicoverload and the legacysetValue+writeCharacteristiccall. Exceptions for permission issues are logged and surfaced asfalse. No further changes needed.
766-903: Helper utilities are robust and defensive
decodePayload,hexToBytes,parseUuidArray,parseUuid,notificationKey,buildScanFilters, andsafeDeviceIdare all straightforward, validate inputs, and handle malformed data gracefully (e.g., hex length/characters, UUID parsing errors). UsingCollections.emptyList()defaults keeps null‑handling simple. This section is in good shape.
919-977: Gatt callback logic and compatibility handling are solid
onConnectionStateChangeadvances the connection state, kicks off service discovery on successful connect, and funnels all disconnects throughfailConnect()to unify error and teardown handling.onServicesDiscoveredmarksservicesDiscovered, flushes queued notifications, logs the layout, and emits a structuredservicespayload. The addition of the deprecated two‑argumentonCharacteristicChangedthat forwards to the three‑argument Android 13+ overload resolves the earlier compatibility gap for Android 12 and below. Notification events include device, service, characteristic, and base64 payload as expected.
979-1028: ScanCriteria matching logic is clear and flexibleThe
ScanCriteriahelper cleanly encodesacceptAll, exact name, prefix (case‑insensitive), and advertised service UUID constraints, short‑circuiting early where possible. TreatingacceptAllas an override that bypasses other filters is a reasonable design choice as long as it matches the JS expectations. No functional problems found.
haslinghuis
force-pushed
the
capacitor-bluetooth-plugin
branch
from
4d85241 to
4af5c2e
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java (1)
961-975: Null safety issue confirmed—add guard against nullvalueinonCharacteristicChangedThe Android Bluetooth API documentation confirms that
BluetoothGattCharacteristic.getValue()can return null due to GATT edge cases, uninitialized cache, or stack-specific behavior (especially on pre-API 33). When the deprecated 2-argonCharacteristicChangedforwardscharacteristic.getValue()to the 3-arg override, a null value reachesBase64.encodeToString(value, ...), causing aNullPointerExceptionand dropping the notification silently.Add a null-safety guard to handle this gracefully:
@Override public void onCharacteristicChanged(BluetoothGatt gatt, BluetoothGattCharacteristic characteristic, byte[] value) { + if (value == null || characteristic == null || characteristic.getService() == null) { + Log.w(TAG, "Notification with null value or characteristic, ignoring"); + return; + } JSObject payload = new JSObject(); payload.put("deviceId", connectedDeviceId); payload.put("service", characteristic.getService().getUuid().toString()); payload.put("characteristic", characteristic.getUuid().toString()); payload.put("value", Base64.encodeToString(value, Base64.NO_WRAP)); notifyListeners("notification", payload); }This prevents crashes on documented Bluetooth stack quirks while keeping normal notifications unchanged.
🧹 Nitpick comments (3)
android/app/src/main/AndroidManifest.xml (1)
58-79: Confirm that hard‑requiring BLE is intendedThe Bluetooth permission set looks aligned with the new plugin, but declaring
android.hardware.bluetooth_lewithandroid:required="true"means the app won’t be installable on devices without BLE, even if USB would still work. If you want “USB‑only but no BLE” devices to remain supported, considerandroid:required="false"here.android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java (2)
176-202: Scan filters still restrict results whenacceptAllDevicesis true
requestDevicemergesservicesandoptionalServicesintocombinedFilter, andstartLeScanalways feedscriteria.serviceUuidsintobuildScanFilters, so Android still filters by service UUID even whenacceptAllDevicesis true. This diverges from the WebBluetooth / PWA pattern where{ acceptAllDevices: true, optionalServices: [...] }shows all devices and does not filter by services at scan time.Since
ScanCriteria.matches()already short‑circuits onacceptAll, a minimal way to align behavior is to skip platform scan filters whenacceptAllis true:- List<ScanFilter> filters = buildScanFilters(criteria.serviceUuids); + // When acceptAll is true, don't restrict by service UUIDs at scan time. + List<ScanFilter> filters = buildScanFilters( + criteria.acceptAll ? Collections.emptyList() : criteria.serviceUuids + );This preserves service‑based filtering when explicitly requested (
acceptAllDevices == false && services.length > 0), while letting the Capacitor path behave like WebBluetooth.js when usingacceptAllDevices+optionalServices.Based on learnings, this would align the CapacitorBluetooth behavior with the existing WebBluetooth.js flow.
Also applies to: 445-484, 904-913, 979-1027
557-576: Disconnect handling can emit duplicate/ambiguousconnectionStateevents
disconnectInternal(false)already callscleanupGatt(), setsconnectionStatetoDISCONNECTED, and sends a"disconnected"connectionStateevent with the last non‑nulldeviceId. Later,onConnectionStateChange(... STATE_DISCONNECTED ...)also callscleanupGatt()andfailConnect(...), which in turn sends anotherconnectionStateevent, but now withdeviceIdcoming fromconnectedDeviceIdafter it has been nulled bycleanupGatt().Result: for an explicit disconnect you can see two events (one “disconnected” with a deviceId, one “Connection failed: Disconnected” with null deviceId), and for remote disconnects the event may carry a null
deviceId.Consider distinguishing “expected disconnect” from “connect failure/remote drop”, and capturing the deviceId before calling
cleanupGatt()in the callback path so listeners always get a consistentdeviceIdfor the last connection. For example, gate thefailConnect(...)call onconnectionState != DISCONNECTING, or split out a dedicated “remoteDisconnect” helper that snapshots the deviceId before cleanup.Also applies to: 596-614, 919-931
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
android/app/src/main/AndroidManifest.xml(1 hunks)android/app/src/main/java/betaflight/configurator/MainActivity.java(1 hunks)android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java(1 hunks)src/js/protocols/CapacitorBluetooth.js(1 hunks)src/js/serial.js(2 hunks)src/js/serial_backend.js(2 hunks)src/js/utils/checkCompatibility.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/js/serial_backend.js
- src/js/serial.js
- src/js/protocols/CapacitorBluetooth.js
- src/js/utils/checkCompatibility.js
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-28T22:51:11.691Z
Learning: In betaflight-configurator, Web Bluetooth API's `services` option filters scan results to only devices advertising those UUIDs, while `optionalServices` requests permission to access services after connection without filtering. PWA (WebBluetooth.js) uses `{ acceptAllDevices: true, optionalServices: uuids }` to show all devices while requesting post-connection service access. CapacitorBluetooth.js should match this pattern rather than using `services` as a scan filter.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:135-143
Timestamp: 2025-11-28T22:41:59.374Z
Learning: In betaflight-configurator, WebSerial.js uses a constant path "serial" for all devices, and CapacitorBluetooth.js uses constant path "bluetooth", consistent with the single-device-at-a-time workflow. WebBluetooth.js uses counter-based paths (bluetooth_${counter}), but that's specific to its implementation. The path strategy varies by protocol based on their specific requirements.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:51:09.253Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported for BLE devices and Web USB API is supported, but Web Serial API is not supported (except limited Bluetooth serial support added in 2025). The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:39:16.388Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported from version 133+ (requires Android 6.0+) and Web USB API is supported from version 136+, but Web Serial API is not supported. The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.
📚 Learning: 2025-06-09T00:32:21.385Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-06-09T00:32:21.385Z
Learning: In the betaflight-configurator codebase, port paths use counter prefixes (e.g., "bluetooth1", "bluetooth2", "serial1") rather than direct protocol identifiers. The protocol selection logic correctly uses `portPath.startsWith("bluetooth")` to detect bluetooth ports regardless of the counter suffix, rather than direct string matching against protocol map keys.
Applied to files:
android/app/src/main/java/betaflight/configurator/MainActivity.javaandroid/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-11-28T22:51:11.691Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-28T22:51:11.691Z
Learning: In betaflight-configurator, Web Bluetooth API's `services` option filters scan results to only devices advertising those UUIDs, while `optionalServices` requests permission to access services after connection without filtering. PWA (WebBluetooth.js) uses `{ acceptAllDevices: true, optionalServices: uuids }` to show all devices while requesting post-connection service access. CapacitorBluetooth.js should match this pattern rather than using `services` as a scan filter.
Applied to files:
android/app/src/main/java/betaflight/configurator/MainActivity.javaandroid/app/src/main/AndroidManifest.xmlandroid/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-05-14T21:51:09.253Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:51:09.253Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported for BLE devices and Web USB API is supported, but Web Serial API is not supported (except limited Bluetooth serial support added in 2025). The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.
Applied to files:
android/app/src/main/java/betaflight/configurator/MainActivity.java
📚 Learning: 2025-11-22T21:18:08.814Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4706
File: src/js/protocols/CapacitorSerial.js:11-46
Timestamp: 2025-11-22T21:18:08.814Z
Learning: In betaflight-configurator, protocol implementations (WebSerial, CapacitorSerial, WebBluetooth, etc.) are instantiated once in the Serial class constructor and stored as singletons in the _protocols array for the entire application lifetime. They are never destroyed or recreated, so cleanup methods to remove event listeners are not needed.
Applied to files:
android/app/src/main/java/betaflight/configurator/MainActivity.java
📚 Learning: 2025-09-19T20:41:44.286Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4510
File: src/js/msp.js:384-391
Timestamp: 2025-09-19T20:41:44.286Z
Learning: When fixing MSP duplicate handling in Betaflight Configurator, avoid complex changes to callback resolution mechanisms as they can break tab switching functionality. Simple duplicate detection based on code and payload size is safer than complex requestKey-based approaches.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-09-19T20:42:20.332Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4510
File: src/js/msp.js:384-391
Timestamp: 2025-09-19T20:42:20.332Z
Learning: Complex MSP duplicate handling fixes in Betaflight Configurator can cause infinite loading messages when changing tabs due to disruption of the callback resolution mechanism. Simple code-only duplicate detection (using this.callbacks.some((instance) => instance.code === code)) is the safer approach that preserves tab switching functionality.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-09-09T20:02:33.475Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4597
File: src/js/utils/common.js:95-127
Timestamp: 2025-09-09T20:02:33.475Z
Learning: In the Betaflight Configurator codebase, the Chromium v140 compatibility issue with sortSelect was resolved by rewriting the implementation to use native DOM APIs (Array.from, select.options, select.remove, select.add) instead of jQuery DOM manipulation methods (this.children, this.empty().append). The fix still performs DOM operations but avoids the specific jQuery methods that were causing issues in Chromium v140.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-06-27T22:06:49.210Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4532
File: src/js/VirtualFC.js:234-234
Timestamp: 2025-06-27T22:06:49.210Z
Learning: In the betaflight-configurator codebase, the VirtualFC.js AUX_CONFIG array must stay synchronized with the betaflight firmware's msp_box.c definitions to ensure proper auxiliary mode functionality. Changes to mode names should follow the firmware source code even if it potentially breaks backward compatibility with saved presets.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-08-27T22:07:26.353Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4579
File: src/js/data_storage.js:9-9
Timestamp: 2025-08-27T22:07:26.353Z
Learning: In the betaflight-configurator CalVer refactoring, the generateVirtualApiVersions function in src/js/utils/common.js works correctly despite the apparent mathematical issue with bounds logic where compareVersions.minor() returns different ranges for API_VERSION_MAX_SUPPORTED (25.12.0) vs API_VERSION_ACCEPTED (1.44.0).
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-08-11T19:10:56.992Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4567
File: locales/en/messages.json:7724-7728
Timestamp: 2025-08-11T19:10:56.992Z
Learning: In the Betaflight Configurator project, only English language entries are added directly to locales/en/messages.json. Translations for other languages are handled through an outsourced process and should not be suggested as part of code reviews.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-09-10T18:26:10.136Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4601
File: package.json:125-125
Timestamp: 2025-09-10T18:26:10.136Z
Learning: In betaflight-configurator, dependency updates are handled incrementally - Vue dependencies are updated separately from Vite dependencies for better isolation and maintainability.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-11-19T17:55:51.714Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4708
File: src/js/tabs/firmware_flasher.js:0-0
Timestamp: 2025-11-19T17:55:51.714Z
Learning: In the betaflight-configurator project, only current Chrome and Edge-based browsers are officially supported, and users are expected to keep their browsers up-to-date for security reasons. This means ES2023 features like Array.prototype.toSorted() and Object.groupBy() can be safely used without polyfills or transpilation.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
🔇 Additional comments (3)
android/app/src/main/java/betaflight/configurator/MainActivity.java (1)
5-13: Bluetooth plugin registration is correct and consistentRegistering
BetaflightBluetoothPluginalongsideBetaflightSerialPluginbeforesuper.onCreatefollows the existing pattern and should integrate cleanly with Capacitor’s bridge lifecycle. No changes needed here.android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java (2)
94-111: Overall plugin structure and thread‑safety look solidUsing
AtomicReferenceforconnectionState,ConcurrentHashMap/ConcurrentLinkedQueuefor shared collections, and a main‑threadHandlerfor GATT entry points gives a good concurrency baseline. Error handling inwrite, notification setup/teardown, and UUID/payload parsing are defensive and clear. Aside from the specific points noted in other comments, the class is well‑structured and fits the existing Capacitor plugin patterns.Also applies to: 167-175, 283-340, 373-443, 771-774, 871-886
204-210: Based on my research into Capacitor plugin architecture, I can now assess the review comment.The web search confirms that Capacitor uses a
setKeepAlive/saveCallpattern for async operations. When aPluginCallis marked as keep-alive and saved, it must be explicitlyresolve()d orreject()d to settle the JavaScript promise. If neither occurs, the promise will hang indefinitely.The review comment correctly identifies this architectural concern: if
requestDevice()keeps the call alive and saves it, butstopScan()only callsstopScanInternal()without settling the promise, the JavaScript promise will indeed remain unresolved.However, since the repository clone failed, I cannot verify:
- The actual implementation of
stopScan()andstopScanInternal()- Whether
pendingScanCallandrejectPendingScan()actually exist- How
requestDevice()manages the call lifecycle- What occurs at lines 515-528 ("Also applies to")
The concern raised is architecturally valid and follows documented Capacitor best practices. The suggested fix (routing through
rejectPendingScan()) is the correct pattern for settling an outstanding async call.
stopScanmay leave the originalrequestDevicecall unresolved if the promise settlement pattern is not followedIn Capacitor's async callback model, when a
PluginCallis kept alive (viasetKeepAlive(true)), it must be explicitly resolved or rejected to settle the JavaScript promise. IfrequestDevice()saves the call andstopScan()callsstopScanInternal()without settling it, the JS promise will hang indefinitely.Route user-initiated stops through
rejectPendingScan(...)to ensure the promise is settled:@PluginMethod public void stopScan(PluginCall call) { - stopScanInternal(); + if (pendingScanCall != null) { + // Cancel the outstanding requestDevice call as well. + rejectPendingScan("Scan stopped"); + } else { + stopScanInternal(); + } JSObject result = new JSObject(); result.put("stopped", true); call.resolve(result); }This keeps internal error/timeout paths unchanged while making
stopScan()behavior predictable for callers.
haslinghuis
force-pushed
the
capacitor-bluetooth-plugin
branch
from
4af5c2e to
619ba7f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/js/protocols/CapacitorBluetooth.js (1)
178-178: Remove unused variable.The
uuidsvariable is computed but never used. Based on past review discussions, theacceptAllDevices: trueworkaround (line 189) is intentional due to UUID mismatch issues on Android, so the uuids computation is no longer needed.Apply this diff:
async requestPermissionDevice() { let newPermissionDevice = null; - const uuids = bluetoothDevices.map((device) => device.serviceUuid).filter(Boolean); try { await BetaflightBluetooth.requestPermissions(); // prompt onceBased on learnings, acceptAllDevices pattern is used because Android devices advertise different service UUIDs than what's in the bluetoothDevices catalog.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
android/app/src/main/AndroidManifest.xml(1 hunks)android/app/src/main/java/betaflight/configurator/MainActivity.java(1 hunks)android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java(1 hunks)src/js/protocols/CapacitorBluetooth.js(1 hunks)src/js/serial.js(2 hunks)src/js/serial_backend.js(2 hunks)src/js/utils/checkCompatibility.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- android/app/src/main/java/betaflight/configurator/MainActivity.java
- src/js/serial_backend.js
- src/js/utils/checkCompatibility.js
- src/js/serial.js
🧰 Additional context used
🧠 Learnings (21)
📓 Common learnings
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-28T22:51:11.691Z
Learning: In betaflight-configurator, Web Bluetooth API's `services` option filters scan results to only devices advertising those UUIDs, while `optionalServices` requests permission to access services after connection without filtering. PWA (WebBluetooth.js) uses `{ acceptAllDevices: true, optionalServices: uuids }` to show all devices while requesting post-connection service access. CapacitorBluetooth.js should match this pattern rather than using `services` as a scan filter.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-30T18:10:26.760Z
Learning: In betaflight-configurator's Android Bluetooth implementation, permission handling must be version-aware: Android 8-11 (API 26-30) requires BLUETOOTH, BLUETOOTH_ADMIN, and ACCESS_COARSE_LOCATION permissions, while Android 12+ (API 31+) requires BLUETOOTH_SCAN and BLUETOOTH_CONNECT. The AndroidManifest.xml should use android:maxSdkVersion="30" on legacy permissions and android:minSdkVersion="31" on new permissions. The Java code must conditionally request permissions using Build.VERSION.SDK_INT >= Build.VERSION_CODES.S check to avoid requesting non-existent permissions on older Android versions.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:51:09.253Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported for BLE devices and Web USB API is supported, but Web Serial API is not supported (except limited Bluetooth serial support added in 2025). The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:39:16.388Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported from version 133+ (requires Android 6.0+) and Web USB API is supported from version 136+, but Web Serial API is not supported. The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:135-143
Timestamp: 2025-11-28T22:41:59.374Z
Learning: In betaflight-configurator, WebSerial.js uses a constant path "serial" for all devices, and CapacitorBluetooth.js uses constant path "bluetooth", consistent with the single-device-at-a-time workflow. WebBluetooth.js uses counter-based paths (bluetooth_${counter}), but that's specific to its implementation. The path strategy varies by protocol based on their specific requirements.
📚 Learning: 2025-11-30T18:10:26.760Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-30T18:10:26.760Z
Learning: In betaflight-configurator's Android Bluetooth implementation, permission handling must be version-aware: Android 8-11 (API 26-30) requires BLUETOOTH, BLUETOOTH_ADMIN, and ACCESS_COARSE_LOCATION permissions, while Android 12+ (API 31+) requires BLUETOOTH_SCAN and BLUETOOTH_CONNECT. The AndroidManifest.xml should use android:maxSdkVersion="30" on legacy permissions and android:minSdkVersion="31" on new permissions. The Java code must conditionally request permissions using Build.VERSION.SDK_INT >= Build.VERSION_CODES.S check to avoid requesting non-existent permissions on older Android versions.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.javaandroid/app/src/main/AndroidManifest.xmlsrc/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-11-28T22:51:11.691Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-28T22:51:11.691Z
Learning: In betaflight-configurator, Web Bluetooth API's `services` option filters scan results to only devices advertising those UUIDs, while `optionalServices` requests permission to access services after connection without filtering. PWA (WebBluetooth.js) uses `{ acceptAllDevices: true, optionalServices: uuids }` to show all devices while requesting post-connection service access. CapacitorBluetooth.js should match this pattern rather than using `services` as a scan filter.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.javaandroid/app/src/main/AndroidManifest.xmlsrc/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-06-09T00:32:21.385Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-06-09T00:32:21.385Z
Learning: In the betaflight-configurator codebase, port paths use counter prefixes (e.g., "bluetooth1", "bluetooth2", "serial1") rather than direct protocol identifiers. The protocol selection logic correctly uses `portPath.startsWith("bluetooth")` to detect bluetooth ports regardless of the counter suffix, rather than direct string matching against protocol map keys.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.javasrc/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-11-28T22:41:59.374Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:135-143
Timestamp: 2025-11-28T22:41:59.374Z
Learning: In betaflight-configurator, WebSerial.js uses a constant path "serial" for all devices, and CapacitorBluetooth.js uses constant path "bluetooth", consistent with the single-device-at-a-time workflow. WebBluetooth.js uses counter-based paths (bluetooth_${counter}), but that's specific to its implementation. The path strategy varies by protocol based on their specific requirements.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.javasrc/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-05-14T21:51:09.253Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:51:09.253Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported for BLE devices and Web USB API is supported, but Web Serial API is not supported (except limited Bluetooth serial support added in 2025). The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.javaandroid/app/src/main/AndroidManifest.xmlsrc/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-09-19T20:41:44.286Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4510
File: src/js/msp.js:384-391
Timestamp: 2025-09-19T20:41:44.286Z
Learning: When fixing MSP duplicate handling in Betaflight Configurator, avoid complex changes to callback resolution mechanisms as they can break tab switching functionality. Simple duplicate detection based on code and payload size is safer than complex requestKey-based approaches.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.javasrc/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-09-19T20:42:20.332Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4510
File: src/js/msp.js:384-391
Timestamp: 2025-09-19T20:42:20.332Z
Learning: Complex MSP duplicate handling fixes in Betaflight Configurator can cause infinite loading messages when changing tabs due to disruption of the callback resolution mechanism. Simple code-only duplicate detection (using this.callbacks.some((instance) => instance.code === code)) is the safer approach that preserves tab switching functionality.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.javasrc/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-09-09T20:02:33.475Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4597
File: src/js/utils/common.js:95-127
Timestamp: 2025-09-09T20:02:33.475Z
Learning: In the Betaflight Configurator codebase, the Chromium v140 compatibility issue with sortSelect was resolved by rewriting the implementation to use native DOM APIs (Array.from, select.options, select.remove, select.add) instead of jQuery DOM manipulation methods (this.children, this.empty().append). The fix still performs DOM operations but avoids the specific jQuery methods that were causing issues in Chromium v140.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-06-27T22:06:49.210Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4532
File: src/js/VirtualFC.js:234-234
Timestamp: 2025-06-27T22:06:49.210Z
Learning: In the betaflight-configurator codebase, the VirtualFC.js AUX_CONFIG array must stay synchronized with the betaflight firmware's msp_box.c definitions to ensure proper auxiliary mode functionality. Changes to mode names should follow the firmware source code even if it potentially breaks backward compatibility with saved presets.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-08-27T22:07:26.353Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4579
File: src/js/data_storage.js:9-9
Timestamp: 2025-08-27T22:07:26.353Z
Learning: In the betaflight-configurator CalVer refactoring, the generateVirtualApiVersions function in src/js/utils/common.js works correctly despite the apparent mathematical issue with bounds logic where compareVersions.minor() returns different ranges for API_VERSION_MAX_SUPPORTED (25.12.0) vs API_VERSION_ACCEPTED (1.44.0).
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-08-11T19:10:56.992Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4567
File: locales/en/messages.json:7724-7728
Timestamp: 2025-08-11T19:10:56.992Z
Learning: In the Betaflight Configurator project, only English language entries are added directly to locales/en/messages.json. Translations for other languages are handled through an outsourced process and should not be suggested as part of code reviews.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-09-10T18:26:10.136Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4601
File: package.json:125-125
Timestamp: 2025-09-10T18:26:10.136Z
Learning: In betaflight-configurator, dependency updates are handled incrementally - Vue dependencies are updated separately from Vite dependencies for better isolation and maintainability.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-11-19T17:55:51.714Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4708
File: src/js/tabs/firmware_flasher.js:0-0
Timestamp: 2025-11-19T17:55:51.714Z
Learning: In the betaflight-configurator project, only current Chrome and Edge-based browsers are officially supported, and users are expected to keep their browsers up-to-date for security reasons. This means ES2023 features like Array.prototype.toSorted() and Object.groupBy() can be safely used without polyfills or transpilation.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-05-14T21:39:16.388Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:39:16.388Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported from version 133+ (requires Android 6.0+) and Web USB API is supported from version 136+, but Web Serial API is not supported. The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.
Applied to files:
android/app/src/main/AndroidManifest.xmlsrc/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-11-24T15:07:25.227Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4706
File: src/js/protocols/CapacitorSerial.js:47-47
Timestamp: 2025-11-24T15:07:25.227Z
Learning: In betaflight-configurator's CapacitorSerial protocol (src/js/protocols/CapacitorSerial.js), the fire-and-forget this.loadDevices() call in the constructor is intentional background pre-loading. The actual critical device list loading for UI population is orchestrated by port_handler.js via serial.getDevices("serial"), which properly awaits CapacitorSerial.getDevices() → loadDevices() in a complete async chain. The constructor's async call does not cause race conditions.
Applied to files:
src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-06-09T00:33:22.959Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-06-09T00:33:22.959Z
Learning: In the betaflight-configurator codebase, port paths use counter suffixes for serial and bluetooth ports (e.g., "serial1", "serial2", "bluetooth1", "bluetooth2") instead of using connectionId, which means simple string matching against protocol map keys won't work for protocol selection.
Applied to files:
src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-06-19T22:13:09.136Z
Learnt from: blckmn
Repo: betaflight/betaflight-configurator PR: 4521
File: src/js/protocols/WebSerial.js:148-151
Timestamp: 2025-06-19T22:13:09.136Z
Learning: In WebSerial.js, there's a timing issue where the cached `this.ports` array doesn't immediately reflect newly permitted devices after `requestPermissionDevice()` completes. The `getDevices()` method needs to refresh the device list from the browser API to return accurate data immediately following a permission request and user acceptance.
Applied to files:
src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-10-25T21:16:32.474Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4379
File: src/js/protocols/TauriSerial.js:203-259
Timestamp: 2025-10-25T21:16:32.474Z
Learning: In TauriSerial (src/js/protocols/TauriSerial.js), the requestPermissionDevice() method is not needed and not invoked. Tauri automatically discovers serial devices through the constructor's loadDevices() and startDeviceMonitoring() calls, bypassing the browser permission model that WebSerial requires. Devices are auto-detected via a 1-second polling interval without user permission prompts.
Applied to files:
src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-11-22T21:18:08.814Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4706
File: src/js/protocols/CapacitorSerial.js:11-46
Timestamp: 2025-11-22T21:18:08.814Z
Learning: In betaflight-configurator, protocol implementations (WebSerial, CapacitorSerial, WebBluetooth, etc.) are instantiated once in the Serial class constructor and stored as singletons in the _protocols array for the entire application lifetime. They are never destroyed or recreated, so cleanup methods to remove event listeners are not needed.
Applied to files:
src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-11-06T17:26:50.201Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4687
File: src/js/protocols/TauriSerial.js:299-310
Timestamp: 2025-11-06T17:26:50.201Z
Learning: In tauri-plugin-serialplugin Android development, the Android project must be initialized via `tauri android init` before the USB support patches (AndroidManifest.xml permissions, device_filter.xml, MainActivity.kt) can be applied. Without initialization, the plugin returns empty results from available_ports_direct() because the generated Android project structure and USB permissions don't exist yet.
Applied to files:
src/js/protocols/CapacitorBluetooth.js
🧬 Code graph analysis (1)
src/js/protocols/CapacitorBluetooth.js (2)
src/js/protocols/devices.js (2)
bluetoothDevices(1-51)bluetoothDevices(1-51)src/js/gui_log.js (1)
gui_log(7-21)
🪛 GitHub Check: SonarCloud Code Analysis
src/js/protocols/CapacitorBluetooth.js
[warning] 178-178: Remove this useless assignment to variable "uuids".
[warning] 41-43: Expected a for-of loop instead of a for loop with this simple iteration.
[warning] 57-57: Prefer String#codePointAt() over String#charCodeAt().
[failure] 87-87: Refactor this asynchronous operation outside of the constructor.
[warning] 42-42: Prefer String.fromCodePoint() over String.fromCharCode().
[warning] 178-178: Remove the declaration of the unused 'uuids' variable.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: deploy
🔇 Additional comments (12)
src/js/protocols/CapacitorBluetooth.js (3)
62-101: LGTM: Constructor follows established patterns.The constructor properly initializes state, attaches native listeners, and follows the singleton pattern established in the codebase. The fire-and-forget listener attachment is appropriate since protocol implementations live for the entire application lifetime.
Based on learnings, protocol implementations are instantiated once and never destroyed, so cleanup methods for event listeners are not needed.
204-276: LGTM: Connection flow with robust service resolution.The connection flow properly:
- Manages connection state flags
- Waits for device characteristics with timeout-based fallback
- Validates complete service/characteristic availability before proceeding
- Starts notifications after successful connection
- Handles errors and cleanup appropriately
The
waitForDeviceCharacteristicspattern with pending promises and timeouts provides robust coordination with the native service discovery events.
375-414: LGTM: Write operation with proper queuing and error handling.The write queue pattern using promise chaining ensures sequential writes while avoiding race conditions. The base64 encoding, payload conversion, and error handling are all appropriate for BLE transport.
android/app/src/main/AndroidManifest.xml (2)
59-77: LGTM: Version-aware Bluetooth permissions properly configured.The permission declarations correctly implement version-specific Bluetooth permission handling:
- Legacy permissions (BLUETOOTH, BLUETOOTH_ADMIN, ACCESS_COARSE_LOCATION) are scoped to Android 8-11 with
maxSdkVersion="30"- New permissions (BLUETOOTH_SCAN, BLUETOOTH_CONNECT) are scoped to Android 12+ with
minSdkVersion="31"- The
neverForLocationflag on BLUETOOTH_SCAN is a good privacy practiceBased on learnings, this structure is necessary because Android 8-11 requires legacy permissions while Android 12+ requires the new runtime permissions, and the Java code must conditionally request permissions using SDK version checks.
68-69: Verify if ACCESS_FINE_LOCATION is necessary for BLE scanning.
ACCESS_FINE_LOCATIONis declared but may not be required. According to Android permission requirements for BLE,ACCESS_COARSE_LOCATIONis typically sufficient for BLE scanning on Android 6-11. Review the Bluetooth implementation code to confirm whether fine location precision is actually needed for your use case, or if coarse location (withmaxSdkVersion="30") would be adequate.android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java (7)
159-242: LGTM: Version-aware permission handling implemented correctly.The permission request implementation properly handles Android version differences:
- Uses
getRequiredPermissions()to determine version-specific permissions- Android 12+ requests BLUETOOTH_SCAN and BLUETOOTH_CONNECT
- Android 8-11 requests legacy BLUETOOTH, BLUETOOTH_ADMIN, and ACCESS_COARSE_LOCATION
- Uses native
ActivityCompat.requestPermissionswith proper callback handling- Stores pending call for later resolution
This matches the learning about version-aware permission handling and avoids requesting non-existent permissions on older Android versions.
115-116: LGTM: Thread-safe collection for pending notification calls.The switch from
ArrayListtoConcurrentLinkedQueueaddresses the race condition identified in previous reviews. The volatileservicesDiscoveredflag combined with the thread-safe queue ensures safe access from multiple threads.
536-604: LGTM: Robust BLE scanning with timeout and criteria matching.The scanning implementation properly:
- Builds scan filters from service UUIDs
- Uses low-latency scan mode for responsive discovery
- Implements scan timeout with cleanup
- Matches devices against name/prefix/service criteria
- Emits device discovery events
- Resolves pending scan call on first match
- Handles scan failures gracefully
The batch result handling and criteria matching logic are well-designed.
622-646: LGTM: GATT connection with proper API level handling.The
openGattmethod correctly:
- Uses
TRANSPORT_LEparameter on Android 6+ (API 23+)- Falls back to two-parameter signature on older versions
- Handles
SecurityExceptionfor missing permissions- Sets up connection timeout with cleanup
- Uses main thread for GATT operations
843-855: LGTM: Write operation with API level compatibility.The write submission handles Android API differences correctly:
- Android 13+ (Tiramisu): Uses new three-parameter
writeCharacteristicAPI- Earlier versions: Uses deprecated two-parameter API with
setValue/setWriteType- Properly handles
SecurityException
1024-1050: LGTM: Service discovery with comprehensive event emission.The
onServicesDiscoveredcallback:
- Sets
servicesDiscoveredflag to unblock pending notification calls- Flushes queued
startNotificationscalls- Builds complete service/characteristic hierarchy with properties
- Emits detailed
servicesevent for JavaScript layerThis provides the JavaScript side with all information needed for dynamic service resolution.
918-943: LGTM: Pending notification call queue with proper lifecycle.The queue management correctly:
- Queues
startNotificationscalls when services aren't discovered yet- Flushes the queue once services are available
- Rejects pending calls on disconnect or connection failure
- Uses thread-safe
ConcurrentLinkedQueueThis ensures notifications can be started immediately after connection without race conditions.
...app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/js/protocols/CapacitorBluetooth.js (1)
36-59: Optional: simplify base64 helpers with modern iteration APIs
toBase64/fromBase64manually build strings with an indexedforloop andString.fromCharCode/charCodeAt. Functionally fine, but you could switch tofor...ofon theUint8ArrayandcodePointAt()for slightly clearer, more idiomatic code if you touch this again. Given the Buffer paths handle most environments, this is purely a readability/style tweak, not a bug.android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java (1)
61-76: Align permission checks with the new runtime permission flow and clean up unused callbackYou’ve moved to explicit runtime permission handling via
getRequiredPermissions()+ActivityCompat.requestPermissions, but:
- The plugin annotation still defines a
bluetoothalias withstrings = {}, andensureBluetoothReady()still checksgetPermissionState("bluetooth"), while@PermissionCallback onPermissionResultis no longer used (you don’t callrequestPermissionForAliasanymore).This means the Capacitor alias state is effectively decoupled from the real OS permission state, and the early “Bluetooth permission not granted” guard may never trigger even if Android permissions are denied.
Consider simplifying and making these consistent by:
- Dropping the alias-based permission path (or leaving the alias empty but not relying on it), and
- Basing readiness purely on your version-aware permission helper:
- private boolean ensureBluetoothReady(PluginCall call) { - if (getPermissionState("bluetooth") != PermissionState.GRANTED) { - call.reject("Bluetooth permission not granted"); - return false; - } + private boolean ensureBluetoothReady(PluginCall call) { + // Check OS permissions using the same logic as requestPermissions() + String[] requiredPermissions = getRequiredPermissions(); + for (String permission : requiredPermissions) { + if (ContextCompat.checkSelfPermission(getContext(), permission) + != PackageManager.PERMISSION_GRANTED) { + call.reject("Bluetooth permission not granted"); + return false; + } + } if (bluetoothAdapter == null) { call.reject("Bluetooth adapter unavailable"); return false; } @@ return true; }And, if you keep this approach, you can remove the now-unused
onPermissionResult@PermissionCallbackmethod and the commented-outrequestPermissionsimplementation to avoid confusion.This will make the permission story clearer and ensure
ensureBluetoothReady()reflects the actual runtime permission state on both Android 8–11 and 12+.Also applies to: 159-210, 244-254, 731-745
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java(1 hunks)src/js/protocols/CapacitorBluetooth.js(1 hunks)
🧰 Additional context used
🧠 Learnings (21)
📓 Common learnings
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-28T22:51:11.691Z
Learning: In betaflight-configurator, Web Bluetooth API's `services` option filters scan results to only devices advertising those UUIDs, while `optionalServices` requests permission to access services after connection without filtering. PWA (WebBluetooth.js) uses `{ acceptAllDevices: true, optionalServices: uuids }` to show all devices while requesting post-connection service access. CapacitorBluetooth.js should match this pattern rather than using `services` as a scan filter.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-30T18:10:26.760Z
Learning: In betaflight-configurator's Android Bluetooth implementation, permission handling must be version-aware: Android 8-11 (API 26-30) requires BLUETOOTH, BLUETOOTH_ADMIN, and ACCESS_COARSE_LOCATION permissions, while Android 12+ (API 31+) requires BLUETOOTH_SCAN and BLUETOOTH_CONNECT. The AndroidManifest.xml should use android:maxSdkVersion="30" on legacy permissions and android:minSdkVersion="31" on new permissions. The Java code must conditionally request permissions using Build.VERSION.SDK_INT >= Build.VERSION_CODES.S check to avoid requesting non-existent permissions on older Android versions.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:51:09.253Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported for BLE devices and Web USB API is supported, but Web Serial API is not supported (except limited Bluetooth serial support added in 2025). The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:39:16.388Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported from version 133+ (requires Android 6.0+) and Web USB API is supported from version 136+, but Web Serial API is not supported. The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:135-143
Timestamp: 2025-11-28T22:41:59.374Z
Learning: In betaflight-configurator, WebSerial.js uses a constant path "serial" for all devices, and CapacitorBluetooth.js uses constant path "bluetooth", consistent with the single-device-at-a-time workflow. WebBluetooth.js uses counter-based paths (bluetooth_${counter}), but that's specific to its implementation. The path strategy varies by protocol based on their specific requirements.
📚 Learning: 2025-11-28T22:51:11.691Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-28T22:51:11.691Z
Learning: In betaflight-configurator, Web Bluetooth API's `services` option filters scan results to only devices advertising those UUIDs, while `optionalServices` requests permission to access services after connection without filtering. PWA (WebBluetooth.js) uses `{ acceptAllDevices: true, optionalServices: uuids }` to show all devices while requesting post-connection service access. CapacitorBluetooth.js should match this pattern rather than using `services` as a scan filter.
Applied to files:
src/js/protocols/CapacitorBluetooth.jsandroid/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-11-28T22:41:59.374Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:135-143
Timestamp: 2025-11-28T22:41:59.374Z
Learning: In betaflight-configurator, WebSerial.js uses a constant path "serial" for all devices, and CapacitorBluetooth.js uses constant path "bluetooth", consistent with the single-device-at-a-time workflow. WebBluetooth.js uses counter-based paths (bluetooth_${counter}), but that's specific to its implementation. The path strategy varies by protocol based on their specific requirements.
Applied to files:
src/js/protocols/CapacitorBluetooth.jsandroid/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-11-24T15:07:25.227Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4706
File: src/js/protocols/CapacitorSerial.js:47-47
Timestamp: 2025-11-24T15:07:25.227Z
Learning: In betaflight-configurator's CapacitorSerial protocol (src/js/protocols/CapacitorSerial.js), the fire-and-forget this.loadDevices() call in the constructor is intentional background pre-loading. The actual critical device list loading for UI population is orchestrated by port_handler.js via serial.getDevices("serial"), which properly awaits CapacitorSerial.getDevices() → loadDevices() in a complete async chain. The constructor's async call does not cause race conditions.
Applied to files:
src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-06-09T00:32:21.385Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-06-09T00:32:21.385Z
Learning: In the betaflight-configurator codebase, port paths use counter prefixes (e.g., "bluetooth1", "bluetooth2", "serial1") rather than direct protocol identifiers. The protocol selection logic correctly uses `portPath.startsWith("bluetooth")` to detect bluetooth ports regardless of the counter suffix, rather than direct string matching against protocol map keys.
Applied to files:
src/js/protocols/CapacitorBluetooth.jsandroid/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-11-22T21:18:08.814Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4706
File: src/js/protocols/CapacitorSerial.js:11-46
Timestamp: 2025-11-22T21:18:08.814Z
Learning: In betaflight-configurator, protocol implementations (WebSerial, CapacitorSerial, WebBluetooth, etc.) are instantiated once in the Serial class constructor and stored as singletons in the _protocols array for the entire application lifetime. They are never destroyed or recreated, so cleanup methods to remove event listeners are not needed.
Applied to files:
src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-06-09T00:33:22.959Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-06-09T00:33:22.959Z
Learning: In the betaflight-configurator codebase, port paths use counter suffixes for serial and bluetooth ports (e.g., "serial1", "serial2", "bluetooth1", "bluetooth2") instead of using connectionId, which means simple string matching against protocol map keys won't work for protocol selection.
Applied to files:
src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-06-19T22:13:09.136Z
Learnt from: blckmn
Repo: betaflight/betaflight-configurator PR: 4521
File: src/js/protocols/WebSerial.js:148-151
Timestamp: 2025-06-19T22:13:09.136Z
Learning: In WebSerial.js, there's a timing issue where the cached `this.ports` array doesn't immediately reflect newly permitted devices after `requestPermissionDevice()` completes. The `getDevices()` method needs to refresh the device list from the browser API to return accurate data immediately following a permission request and user acceptance.
Applied to files:
src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-10-25T21:16:32.474Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4379
File: src/js/protocols/TauriSerial.js:203-259
Timestamp: 2025-10-25T21:16:32.474Z
Learning: In TauriSerial (src/js/protocols/TauriSerial.js), the requestPermissionDevice() method is not needed and not invoked. Tauri automatically discovers serial devices through the constructor's loadDevices() and startDeviceMonitoring() calls, bypassing the browser permission model that WebSerial requires. Devices are auto-detected via a 1-second polling interval without user permission prompts.
Applied to files:
src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-09-19T20:41:44.286Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4510
File: src/js/msp.js:384-391
Timestamp: 2025-09-19T20:41:44.286Z
Learning: When fixing MSP duplicate handling in Betaflight Configurator, avoid complex changes to callback resolution mechanisms as they can break tab switching functionality. Simple duplicate detection based on code and payload size is safer than complex requestKey-based approaches.
Applied to files:
src/js/protocols/CapacitorBluetooth.jsandroid/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-09-19T20:42:20.332Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4510
File: src/js/msp.js:384-391
Timestamp: 2025-09-19T20:42:20.332Z
Learning: Complex MSP duplicate handling fixes in Betaflight Configurator can cause infinite loading messages when changing tabs due to disruption of the callback resolution mechanism. Simple code-only duplicate detection (using this.callbacks.some((instance) => instance.code === code)) is the safer approach that preserves tab switching functionality.
Applied to files:
src/js/protocols/CapacitorBluetooth.jsandroid/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-05-14T21:51:09.253Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:51:09.253Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported for BLE devices and Web USB API is supported, but Web Serial API is not supported (except limited Bluetooth serial support added in 2025). The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.
Applied to files:
src/js/protocols/CapacitorBluetooth.jsandroid/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-05-14T21:39:16.388Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:39:16.388Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported from version 133+ (requires Android 6.0+) and Web USB API is supported from version 136+, but Web Serial API is not supported. The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.
Applied to files:
src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-11-30T18:10:26.760Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-30T18:10:26.760Z
Learning: In betaflight-configurator's Android Bluetooth implementation, permission handling must be version-aware: Android 8-11 (API 26-30) requires BLUETOOTH, BLUETOOTH_ADMIN, and ACCESS_COARSE_LOCATION permissions, while Android 12+ (API 31+) requires BLUETOOTH_SCAN and BLUETOOTH_CONNECT. The AndroidManifest.xml should use android:maxSdkVersion="30" on legacy permissions and android:minSdkVersion="31" on new permissions. The Java code must conditionally request permissions using Build.VERSION.SDK_INT >= Build.VERSION_CODES.S check to avoid requesting non-existent permissions on older Android versions.
Applied to files:
src/js/protocols/CapacitorBluetooth.jsandroid/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-11-06T17:26:50.201Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4687
File: src/js/protocols/TauriSerial.js:299-310
Timestamp: 2025-11-06T17:26:50.201Z
Learning: In tauri-plugin-serialplugin Android development, the Android project must be initialized via `tauri android init` before the USB support patches (AndroidManifest.xml permissions, device_filter.xml, MainActivity.kt) can be applied. Without initialization, the plugin returns empty results from available_ports_direct() because the generated Android project structure and USB permissions don't exist yet.
Applied to files:
src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-09-09T20:02:33.475Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4597
File: src/js/utils/common.js:95-127
Timestamp: 2025-09-09T20:02:33.475Z
Learning: In the Betaflight Configurator codebase, the Chromium v140 compatibility issue with sortSelect was resolved by rewriting the implementation to use native DOM APIs (Array.from, select.options, select.remove, select.add) instead of jQuery DOM manipulation methods (this.children, this.empty().append). The fix still performs DOM operations but avoids the specific jQuery methods that were causing issues in Chromium v140.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-06-27T22:06:49.210Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4532
File: src/js/VirtualFC.js:234-234
Timestamp: 2025-06-27T22:06:49.210Z
Learning: In the betaflight-configurator codebase, the VirtualFC.js AUX_CONFIG array must stay synchronized with the betaflight firmware's msp_box.c definitions to ensure proper auxiliary mode functionality. Changes to mode names should follow the firmware source code even if it potentially breaks backward compatibility with saved presets.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-08-27T22:07:26.353Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4579
File: src/js/data_storage.js:9-9
Timestamp: 2025-08-27T22:07:26.353Z
Learning: In the betaflight-configurator CalVer refactoring, the generateVirtualApiVersions function in src/js/utils/common.js works correctly despite the apparent mathematical issue with bounds logic where compareVersions.minor() returns different ranges for API_VERSION_MAX_SUPPORTED (25.12.0) vs API_VERSION_ACCEPTED (1.44.0).
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-08-11T19:10:56.992Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4567
File: locales/en/messages.json:7724-7728
Timestamp: 2025-08-11T19:10:56.992Z
Learning: In the Betaflight Configurator project, only English language entries are added directly to locales/en/messages.json. Translations for other languages are handled through an outsourced process and should not be suggested as part of code reviews.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-09-10T18:26:10.136Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4601
File: package.json:125-125
Timestamp: 2025-09-10T18:26:10.136Z
Learning: In betaflight-configurator, dependency updates are handled incrementally - Vue dependencies are updated separately from Vite dependencies for better isolation and maintainability.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-11-19T17:55:51.714Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4708
File: src/js/tabs/firmware_flasher.js:0-0
Timestamp: 2025-11-19T17:55:51.714Z
Learning: In the betaflight-configurator project, only current Chrome and Edge-based browsers are officially supported, and users are expected to keep their browsers up-to-date for security reasons. This means ES2023 features like Array.prototype.toSorted() and Object.groupBy() can be safely used without polyfills or transpilation.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
🧬 Code graph analysis (1)
src/js/protocols/CapacitorBluetooth.js (2)
src/js/gui_log.js (1)
gui_log(7-21)src/js/protocols/devices.js (2)
bluetoothDevices(1-51)bluetoothDevices(1-51)
🪛 GitHub Actions: Deployment (PR and Push)
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
[error] 1055-1055: Compilation failed: cannot find symbol 'value' referenced in 'if (characteristic == null || value == null) {'. Likely 'value' is undefined or out of scope in this context (BetaflightBluetoothPlugin.java:1055).
🪛 GitHub Check: SonarCloud Code Analysis
src/js/protocols/CapacitorBluetooth.js
[failure] 87-87: Refactor this asynchronous operation outside of the constructor.
[warning] 42-42: Prefer String.fromCodePoint() over String.fromCharCode().
[warning] 41-43: Expected a for-of loop instead of a for loop with this simple iteration.
[warning] 57-57: Prefer String#codePointAt() over String#charCodeAt().
🔇 Additional comments (1)
src/js/protocols/CapacitorBluetooth.js (1)
62-647: Capacitor Bluetooth adapter design looks solid and consistent with existing protocolsConnection lifecycle, dynamic service/characteristic resolution, CRC workaround, serialized write queue, and native listener wiring all look coherent and in line with the existing Serial/WebBluetooth patterns. No blocking issues from this side.
...app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java (1)
1050-1062: Add null check for characteristic.getService().Line 1058 calls
characteristic.getService().getUuid()without checking ifgetService()returns null. While the code checkscharacteristic == nullat line 1052, it doesn't verify thatgetService()is non-null before callinggetUuid()on it.Apply this diff to add the missing null check:
@Override public void onCharacteristicChanged(BluetoothGatt gatt, BluetoothGattCharacteristic characteristic, byte[] value) { - if (characteristic == null || value == null) { + if (characteristic == null || characteristic.getService() == null || value == null) { Log.w(TAG, "Received notification with null characteristic or value"); return; } JSObject payload = new JSObject(); payload.put("deviceId", connectedDeviceId); payload.put("service", characteristic.getService().getUuid().toString()); payload.put("characteristic", characteristic.getUuid().toString()); payload.put("value", Base64.encodeToString(value, Base64.NO_WRAP)); notifyListeners("notification", payload); }
🧹 Nitpick comments (5)
src/js/msp/MSPHelper.js (1)
2935-2960: Save & Reboot queue quiescing looks correct; consider adding a timeout fallback.The new
rebootguard that callsGUI.interval_kill_all()andMSP.callbacks_cleanup()beforeMSP_EEPROM_WRITE, plus the increased delay to 200 ms, is a good way to reduce MSP queue pressure and stale callbacks during Save & Reboot (especially over BLE). This is a safe change as it only applies whenreboot === trueand doesn’t affect plain “save only” flows.One remaining robustness gap is that, if the EEPROM write callback is still lost (e.g. link glitch), the UI will wait indefinitely for
GUI.tab_switch_cleanup→reinitializeConnection. You may want to add an optional timeout (on the JS side) that, after e.g. ~3 s withoutMSP_EEPROM_WRITE, proceeds withtab_switch_cleanup/reinitializeConnectionanyway for reboot flows, while still preferring the real ack when it arrives. This would harden Save & Reboot on slow or lossy BLE links without changing behavior for fast paths.src/js/protocols/CapacitorBluetooth.js (3)
102-143: Device handling and permission flow are reasonable; just ensure UUIDs are always present.
requestPermissionDevice()→handleNewDevice()correctly gates onBetaflightBluetooth.requestPermissions()and then callsrequestDevice({ acceptAllDevices: true }), which matches the current Android plugin behavior. The device list is de‑duplicated bydeviceId, and ports are created viacreatePort()with a constant"bluetooth"path, consistent with the existing serial protocol pattern (single active device at a time, protocol‑specific mapping elsewhere). Based on learnings, this aligns with how other protocols behave.One thing to keep in mind is that
handleNewDevice()and the final return inrequestPermissionDevice()both requiredevice.uuids?.lengthto be truthy; devices for which the plugin does not populateuuidswill be silently discarded and never exposed to the UI. If the Android side guaranteesuuidsis always filled for supported hardware, this is fine; otherwise, consider relaxing the UUID requirement and falling back to name‑based resolution only whenuuidsis missing.Also applies to: 176-202
379-418: Sequential write queue is sound; a targeted retry on transient write errors is optional.The
send()method:
- Validates connection and presence of
writeCharacteristic,- Normalizes payload via
toUint8Array,- Serializes to base64 once,
- Queues writes through
this.writeQueueso only one BLE write is in flight at a time,- Resets the queue even on failure (
catchonwriteTask) to avoid permanent blockage,- Updates
bytesSentonly on success and surfaces errors via both return value and optional callback.This is a solid pattern for BLE writes. If you still occasionally see the “Unable to write characteristic” error from the Android side on the first write after connect, you could optionally add a single short retry inside the queued write block (e.g. retry once after a small delay only for that specific error string). That would further harden the path without changing behavior for other failures.
345-377: Disconnect handling is robust; consider guarding against duplicate “disconnect” events.
disconnect()callsstopNotifications(), thenBetaflightBluetooth.disconnect(), and finallyhandleRemoteDisconnect(targetDeviceId, true)to ensure local cleanup even if the native side doesn’t emit a connection‑state event. Separately, both theconnectionStatelistener and theconnect()callback can callhandleRemoteDisconnect(deviceId)for remote disconnects.Because
handleRemoteDisconnect()only short‑circuits whendisconnectHandled && !forced, a local disconnect that also triggers aconnectionStateevent could, in theory, result in two"disconnect"events being dispatched (one from the native event, one from theforcedcall). If downstream consumers treat"disconnect"as idempotent this is harmless, but you might want to tighten this by skipping the forced call whendisconnectHandledhas already flipped to true after a native event.Also applies to: 420-458
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java (1)
264-267: Consider optionalServices semantics for scanning.Lines 264-267 combine both
servicesandoptionalServicesinto a singlecombinedFilterthat is passed to the scan criteria. This combined filter is then used to build scan filters at line 527.According to Web Bluetooth API semantics,
optionalServicesshould only request permission to access services after connection without filtering scan results, whileservicesshould filter which devices are shown during scanning.If the intent is to match Web Bluetooth behavior (as mentioned in retrieved learnings), consider separating these concerns:
- Use only
servicesfor scan filtering- Store
optionalServicesseparately for post-connection service access permissionBased on learnings: "PWA (WebBluetooth.js) uses
{ acceptAllDevices: true, optionalServices: uuids }to show all devices while requesting post-connection service access. CapacitorBluetooth.js should match this pattern rather than usingservicesas a scan filter."
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java(1 hunks)src/js/msp/MSPHelper.js(2 hunks)src/js/protocols/CapacitorBluetooth.js(1 hunks)
🧰 Additional context used
🧠 Learnings (25)
📓 Common learnings
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-01T17:09:13.412Z
Learning: In betaflight-configurator’s CapacitorBluetooth.js, native plugin listeners (notification, services, connectionState) must be attached exactly once per app lifetime. Re-attaching them during writes/connect cycles leads to duplicate handlers, increased JS workload, MSP queue pressure, and can prevent the MSP_EEPROM_WRITE callback from firing (so reinitializeConnection is not reached). Implement an idempotent _attachNativeListenersOnce() guard.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-28T22:51:11.691Z
Learning: In betaflight-configurator, Web Bluetooth API's `services` option filters scan results to only devices advertising those UUIDs, while `optionalServices` requests permission to access services after connection without filtering. PWA (WebBluetooth.js) uses `{ acceptAllDevices: true, optionalServices: uuids }` to show all devices while requesting post-connection service access. CapacitorBluetooth.js should match this pattern rather than using `services` as a scan filter.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-01T17:09:13.412Z
Learning: CapacitorBluetooth.connect should only dispatch the 'open' event after GATT services are resolved and notifications are started; announcing open earlier causes the first MSP write to fail with “Unable to write characteristic” and contributes to early queue load.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-01T17:38:41.581Z
Learning: On BLE (Android) Save & Reboot, status pull intervals continue during MSP_EEPROM_WRITE, overwhelming the MSP queue and causing the EEPROM ack to time out so reinitializeConnection never runs. Solution: before EEPROM write, call GUI.interval_kill_all() and MSP.callbacks_cleanup(), and add a 3s fallback to proceed to reboot if the ack is missed.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4706
File: src/js/protocols/CapacitorSerial.js:47-47
Timestamp: 2025-11-24T15:07:25.227Z
Learning: In betaflight-configurator's CapacitorSerial protocol (src/js/protocols/CapacitorSerial.js), the fire-and-forget this.loadDevices() call in the constructor is intentional background pre-loading. The actual critical device list loading for UI population is orchestrated by port_handler.js via serial.getDevices("serial"), which properly awaits CapacitorSerial.getDevices() → loadDevices() in a complete async chain. The constructor's async call does not cause race conditions.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4510
File: src/js/msp.js:384-391
Timestamp: 2025-09-19T20:41:44.286Z
Learning: When fixing MSP duplicate handling in Betaflight Configurator, avoid complex changes to callback resolution mechanisms as they can break tab switching functionality. Simple duplicate detection based on code and payload size is safer than complex requestKey-based approaches.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:135-143
Timestamp: 2025-11-28T22:41:59.374Z
Learning: In betaflight-configurator, WebSerial.js uses a constant path "serial" for all devices, and CapacitorBluetooth.js uses constant path "bluetooth", consistent with the single-device-at-a-time workflow. WebBluetooth.js uses counter-based paths (bluetooth_${counter}), but that's specific to its implementation. The path strategy varies by protocol based on their specific requirements.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-06-09T00:32:21.385Z
Learning: In the betaflight-configurator codebase, port paths use counter prefixes (e.g., "bluetooth1", "bluetooth2", "serial1") rather than direct protocol identifiers. The protocol selection logic correctly uses `portPath.startsWith("bluetooth")` to detect bluetooth ports regardless of the counter suffix, rather than direct string matching against protocol map keys.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-30T18:10:26.784Z
Learning: In betaflight-configurator's Android Bluetooth implementation, permission handling must be version-aware: Android 8-11 (API 26-30) requires BLUETOOTH, BLUETOOTH_ADMIN, and ACCESS_COARSE_LOCATION permissions, while Android 12+ (API 31+) requires BLUETOOTH_SCAN and BLUETOOTH_CONNECT. The AndroidManifest.xml should use android:maxSdkVersion="30" on legacy permissions and android:minSdkVersion="31" on new permissions. The Java code must conditionally request permissions using Build.VERSION.SDK_INT >= Build.VERSION_CODES.S check to avoid requesting non-existent permissions on older Android versions.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:51:09.253Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported for BLE devices and Web USB API is supported, but Web Serial API is not supported (except limited Bluetooth serial support added in 2025). The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:39:16.388Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported from version 133+ (requires Android 6.0+) and Web USB API is supported from version 136+, but Web Serial API is not supported. The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.
📚 Learning: 2025-11-28T22:51:11.691Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-28T22:51:11.691Z
Learning: In betaflight-configurator, Web Bluetooth API's `services` option filters scan results to only devices advertising those UUIDs, while `optionalServices` requests permission to access services after connection without filtering. PWA (WebBluetooth.js) uses `{ acceptAllDevices: true, optionalServices: uuids }` to show all devices while requesting post-connection service access. CapacitorBluetooth.js should match this pattern rather than using `services` as a scan filter.
Applied to files:
src/js/protocols/CapacitorBluetooth.jsandroid/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-12-01T17:09:13.412Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-01T17:09:13.412Z
Learning: In betaflight-configurator’s CapacitorBluetooth.js, native plugin listeners (notification, services, connectionState) must be attached exactly once per app lifetime. Re-attaching them during writes/connect cycles leads to duplicate handlers, increased JS workload, MSP queue pressure, and can prevent the MSP_EEPROM_WRITE callback from firing (so reinitializeConnection is not reached). Implement an idempotent _attachNativeListenersOnce() guard.
Applied to files:
src/js/protocols/CapacitorBluetooth.jsandroid/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-11-28T22:41:59.374Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:135-143
Timestamp: 2025-11-28T22:41:59.374Z
Learning: In betaflight-configurator, WebSerial.js uses a constant path "serial" for all devices, and CapacitorBluetooth.js uses constant path "bluetooth", consistent with the single-device-at-a-time workflow. WebBluetooth.js uses counter-based paths (bluetooth_${counter}), but that's specific to its implementation. The path strategy varies by protocol based on their specific requirements.
Applied to files:
src/js/protocols/CapacitorBluetooth.jsandroid/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-11-24T15:07:25.227Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4706
File: src/js/protocols/CapacitorSerial.js:47-47
Timestamp: 2025-11-24T15:07:25.227Z
Learning: In betaflight-configurator's CapacitorSerial protocol (src/js/protocols/CapacitorSerial.js), the fire-and-forget this.loadDevices() call in the constructor is intentional background pre-loading. The actual critical device list loading for UI population is orchestrated by port_handler.js via serial.getDevices("serial"), which properly awaits CapacitorSerial.getDevices() → loadDevices() in a complete async chain. The constructor's async call does not cause race conditions.
Applied to files:
src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-12-01T17:09:13.412Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-01T17:09:13.412Z
Learning: CapacitorBluetooth.connect should only dispatch the 'open' event after GATT services are resolved and notifications are started; announcing open earlier causes the first MSP write to fail with “Unable to write characteristic” and contributes to early queue load.
Applied to files:
src/js/protocols/CapacitorBluetooth.jsandroid/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-06-09T00:32:21.385Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-06-09T00:32:21.385Z
Learning: In the betaflight-configurator codebase, port paths use counter prefixes (e.g., "bluetooth1", "bluetooth2", "serial1") rather than direct protocol identifiers. The protocol selection logic correctly uses `portPath.startsWith("bluetooth")` to detect bluetooth ports regardless of the counter suffix, rather than direct string matching against protocol map keys.
Applied to files:
src/js/protocols/CapacitorBluetooth.jsandroid/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-11-22T21:18:08.814Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4706
File: src/js/protocols/CapacitorSerial.js:11-46
Timestamp: 2025-11-22T21:18:08.814Z
Learning: In betaflight-configurator, protocol implementations (WebSerial, CapacitorSerial, WebBluetooth, etc.) are instantiated once in the Serial class constructor and stored as singletons in the _protocols array for the entire application lifetime. They are never destroyed or recreated, so cleanup methods to remove event listeners are not needed.
Applied to files:
src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-06-09T00:33:22.959Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-06-09T00:33:22.959Z
Learning: In the betaflight-configurator codebase, port paths use counter suffixes for serial and bluetooth ports (e.g., "serial1", "serial2", "bluetooth1", "bluetooth2") instead of using connectionId, which means simple string matching against protocol map keys won't work for protocol selection.
Applied to files:
src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-06-19T22:13:09.136Z
Learnt from: blckmn
Repo: betaflight/betaflight-configurator PR: 4521
File: src/js/protocols/WebSerial.js:148-151
Timestamp: 2025-06-19T22:13:09.136Z
Learning: In WebSerial.js, there's a timing issue where the cached `this.ports` array doesn't immediately reflect newly permitted devices after `requestPermissionDevice()` completes. The `getDevices()` method needs to refresh the device list from the browser API to return accurate data immediately following a permission request and user acceptance.
Applied to files:
src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-10-25T21:16:32.474Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4379
File: src/js/protocols/TauriSerial.js:203-259
Timestamp: 2025-10-25T21:16:32.474Z
Learning: In TauriSerial (src/js/protocols/TauriSerial.js), the requestPermissionDevice() method is not needed and not invoked. Tauri automatically discovers serial devices through the constructor's loadDevices() and startDeviceMonitoring() calls, bypassing the browser permission model that WebSerial requires. Devices are auto-detected via a 1-second polling interval without user permission prompts.
Applied to files:
src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-09-19T20:41:44.286Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4510
File: src/js/msp.js:384-391
Timestamp: 2025-09-19T20:41:44.286Z
Learning: When fixing MSP duplicate handling in Betaflight Configurator, avoid complex changes to callback resolution mechanisms as they can break tab switching functionality. Simple duplicate detection based on code and payload size is safer than complex requestKey-based approaches.
Applied to files:
src/js/protocols/CapacitorBluetooth.jsandroid/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-09-19T20:42:20.332Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4510
File: src/js/msp.js:384-391
Timestamp: 2025-09-19T20:42:20.332Z
Learning: Complex MSP duplicate handling fixes in Betaflight Configurator can cause infinite loading messages when changing tabs due to disruption of the callback resolution mechanism. Simple code-only duplicate detection (using this.callbacks.some((instance) => instance.code === code)) is the safer approach that preserves tab switching functionality.
Applied to files:
src/js/protocols/CapacitorBluetooth.jssrc/js/msp/MSPHelper.jsandroid/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-05-14T21:51:09.253Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:51:09.253Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported for BLE devices and Web USB API is supported, but Web Serial API is not supported (except limited Bluetooth serial support added in 2025). The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.
Applied to files:
src/js/protocols/CapacitorBluetooth.jsandroid/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-05-14T21:39:16.388Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:39:16.388Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported from version 133+ (requires Android 6.0+) and Web USB API is supported from version 136+, but Web Serial API is not supported. The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.
Applied to files:
src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-11-30T18:10:26.784Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-30T18:10:26.784Z
Learning: In betaflight-configurator's Android Bluetooth implementation, permission handling must be version-aware: Android 8-11 (API 26-30) requires BLUETOOTH, BLUETOOTH_ADMIN, and ACCESS_COARSE_LOCATION permissions, while Android 12+ (API 31+) requires BLUETOOTH_SCAN and BLUETOOTH_CONNECT. The AndroidManifest.xml should use android:maxSdkVersion="30" on legacy permissions and android:minSdkVersion="31" on new permissions. The Java code must conditionally request permissions using Build.VERSION.SDK_INT >= Build.VERSION_CODES.S check to avoid requesting non-existent permissions on older Android versions.
Applied to files:
src/js/protocols/CapacitorBluetooth.jsandroid/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-11-06T17:26:50.201Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4687
File: src/js/protocols/TauriSerial.js:299-310
Timestamp: 2025-11-06T17:26:50.201Z
Learning: In tauri-plugin-serialplugin Android development, the Android project must be initialized via `tauri android init` before the USB support patches (AndroidManifest.xml permissions, device_filter.xml, MainActivity.kt) can be applied. Without initialization, the plugin returns empty results from available_ports_direct() because the generated Android project structure and USB permissions don't exist yet.
Applied to files:
src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-12-01T17:38:41.581Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-01T17:38:41.581Z
Learning: On BLE (Android) Save & Reboot, status pull intervals continue during MSP_EEPROM_WRITE, overwhelming the MSP queue and causing the EEPROM ack to time out so reinitializeConnection never runs. Solution: before EEPROM write, call GUI.interval_kill_all() and MSP.callbacks_cleanup(), and add a 3s fallback to proceed to reboot if the ack is missed.
Applied to files:
src/js/msp/MSPHelper.js
📚 Learning: 2025-08-27T19:36:13.733Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4579
File: src/js/tabs/pid_tuning.js:80-82
Timestamp: 2025-08-27T19:36:13.733Z
Learning: MSP_STATUS_EX calls in serial_backend.js and setup.js are intentionally unguarded because they are used for status fetching on page load to initialize profiles and are safe to call on older firmware versions. Only conditional/optional MSP_STATUS_EX usage (like in pid_tuning.js) needs version gating to API_VERSION_25_12.
Applied to files:
src/js/msp/MSPHelper.js
📚 Learning: 2025-09-09T20:02:33.475Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4597
File: src/js/utils/common.js:95-127
Timestamp: 2025-09-09T20:02:33.475Z
Learning: In the Betaflight Configurator codebase, the Chromium v140 compatibility issue with sortSelect was resolved by rewriting the implementation to use native DOM APIs (Array.from, select.options, select.remove, select.add) instead of jQuery DOM manipulation methods (this.children, this.empty().append). The fix still performs DOM operations but avoids the specific jQuery methods that were causing issues in Chromium v140.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-06-27T22:06:49.210Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4532
File: src/js/VirtualFC.js:234-234
Timestamp: 2025-06-27T22:06:49.210Z
Learning: In the betaflight-configurator codebase, the VirtualFC.js AUX_CONFIG array must stay synchronized with the betaflight firmware's msp_box.c definitions to ensure proper auxiliary mode functionality. Changes to mode names should follow the firmware source code even if it potentially breaks backward compatibility with saved presets.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-08-27T22:07:26.353Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4579
File: src/js/data_storage.js:9-9
Timestamp: 2025-08-27T22:07:26.353Z
Learning: In the betaflight-configurator CalVer refactoring, the generateVirtualApiVersions function in src/js/utils/common.js works correctly despite the apparent mathematical issue with bounds logic where compareVersions.minor() returns different ranges for API_VERSION_MAX_SUPPORTED (25.12.0) vs API_VERSION_ACCEPTED (1.44.0).
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-08-11T19:10:56.992Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4567
File: locales/en/messages.json:7724-7728
Timestamp: 2025-08-11T19:10:56.992Z
Learning: In the Betaflight Configurator project, only English language entries are added directly to locales/en/messages.json. Translations for other languages are handled through an outsourced process and should not be suggested as part of code reviews.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-09-10T18:26:10.136Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4601
File: package.json:125-125
Timestamp: 2025-09-10T18:26:10.136Z
Learning: In betaflight-configurator, dependency updates are handled incrementally - Vue dependencies are updated separately from Vite dependencies for better isolation and maintainability.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-11-19T17:55:51.714Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4708
File: src/js/tabs/firmware_flasher.js:0-0
Timestamp: 2025-11-19T17:55:51.714Z
Learning: In the betaflight-configurator project, only current Chrome and Edge-based browsers are officially supported, and users are expected to keep their browsers up-to-date for security reasons. This means ES2023 features like Array.prototype.toSorted() and Object.groupBy() can be safely used without polyfills or transpilation.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
🧬 Code graph analysis (1)
src/js/protocols/CapacitorBluetooth.js (1)
src/js/gui_log.js (1)
gui_log(7-21)
🪛 GitHub Check: SonarCloud Code Analysis
src/js/protocols/CapacitorBluetooth.js
[failure] 87-87: Refactor this asynchronous operation outside of the constructor.
[warning] 57-57: Prefer String#codePointAt() over String#charCodeAt().
[warning] 42-42: Prefer String.fromCodePoint() over String.fromCharCode().
[warning] 41-43: Expected a for-of loop instead of a for loop with this simple iteration.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (19)
src/js/protocols/CapacitorBluetooth.js (4)
10-60: Utility conversions for UUIDs and base64/Uint8Array look solid.
toLowerUuid,toUint8Array,toBase64, andfromBase64handle the expected input shapes and both browser/Node environments (Buffer vs btoa/atob) without obvious edge‑case issues. No changes needed here.
62-100: Async listener setup in the constructor is acceptable given singleton lifetime.
CapacitorBluetoothinstances are singletons for the app lifetime (same pattern asCapacitorSerial), andattachNativeListeners()is idempotent vianativeListenersReady. Triggering listener registration from the constructor is effectively a one‑time fire‑and‑forget setup and doesn’t introduce observable races in the protocol API.Sonar’s “async operation in constructor” warning can be treated as a false positive in this context; I wouldn’t refactor this unless you hit an actual lifecycle issue.
Also applies to: 599-649
208-280: Connect flow correctly waits for characteristics/notifications before signaling “open”.The
connect()implementation:
- Finds the requested port, resolves a static
deviceDescriptionfrombluetoothDevices,- Calls
BetaflightBluetooth.connect()with a remote‑disconnect callback,- Awaits
waitForDeviceCharacteristics()(which uses discovered services + static profile),- Verifies
hasCompleteCharacteristics(),- Then calls
startNotifications()before settingconnectedand emitting the"connect"event.This ordering matches the prior requirement that the “open” signal should only be emitted once GATT services are resolved and notifications are running, which helps avoid the initial “Unable to write characteristic” error and early MSP queue load. The pending‑resolver/timeout logic for service discovery is also well‑contained, and
cleanupConnectionState()+clearPendingServiceResolver()ensure we don’t leak pending promises on failures or disconnects.No changes needed here.
Also applies to: 464-522, 567-584
524-597: Service discovery and description merging are well-structured.
buildDescriptionFromServices()correctly derives a service UUID and suitable write/notify characteristics from the GATT descriptor, honoring the GATT properties bitmask. The combination of:
discoveredServicescache perdeviceId,pendingServiceResolverswith an 8 s timeout,mergeDeviceDescription()to overlay dynamically discovered characteristics on top of the staticbluetoothDevicesentry,gives you a flexible fallback path while avoiding orphaned promises when devices disconnect or never emit a
servicesevent. This looks good as-is.android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java (15)
71-110: LGTM! Thread-safe state management.The plugin's state management properly uses thread-safe collections (ConcurrentHashMap, ConcurrentLinkedQueue, AtomicReference) and volatile for shared state accessed from multiple threads (GATT callback thread, request threads, main thread). This addresses the earlier race condition concerns.
111-129: LGTM! Proper lifecycle management.The plugin correctly initializes Bluetooth components in
load()and ensures cleanup inhandleOnDestroy()by stopping active scans and disconnecting.
283-324: LGTM! Clean connection state management.The connection flow properly:
- Validates device and Bluetooth readiness
- Uses atomic compareAndSet for thread-safe state transition
- Clears pending notifications and resets service discovery state before connecting
- Delegates to main thread for GATT operations
326-411: LGTM! Robust write operation with proper sequencing.The write method includes:
- Thorough validation of UUIDs, value, and connection state
- Fallback search across all services if characteristic not found in specified service
- Proper write type selection based on characteristic properties
- Multiple encoding support (hex/utf8/base64)
The connect flow correctly waits for service discovery before resolving (line 1031), which addresses the learning about announcing 'open' only after GATT services are resolved.
Based on learnings: "CapacitorBluetooth.connect should only dispatch the 'open' event after GATT services are resolved and notifications are started."
413-514: LGTM! Proper notification lifecycle management.The notification handling correctly:
- Queues
startNotificationscalls until services are discovered (lines 419-422)- Flushes queued calls after service discovery (lines 904-912)
- Maintains thread-safe activeNotifications tracking
- Includes fallback service lookup
- Properly enables/disables GATT notifications
516-599: LGTM! Complete scan lifecycle with timeout protection.The scan implementation properly:
- Validates Bluetooth components before scanning
- Builds service UUID filters or accepts all devices
- Implements timeout with proper cleanup
- Handles individual and batch scan results
- Resolves pending scan request on first matching device
- Cleans up resources in stopScanInternal
601-626: LGTM! Version-aware GATT connection with proper error handling.The GATT connection correctly:
- Uses API level check for transport parameter (API 23+)
- Catches SecurityException for missing BLUETOOTH_CONNECT permission
- Sets connection timeout (12 seconds)
- Validates device before attempting connection
628-709: LGTM! Comprehensive connection lifecycle management.The connection management helpers properly:
- Handle success and failure paths with appropriate cleanup
- Reset state atomically with proper threading
- Reject pending calls and notify listeners
- Clean up GATT resources with exception handling
- Track and communicate connection state changes
711-736: LGTM! Clear validation with informative errors.The validation helpers provide proper checks for Bluetooth readiness and connection state with clear rejection messages for callers.
738-778: LGTM! Device resolution with cache and fallback.Device resolution and payload creation properly:
- Checks discoveredDevices cache first
- Falls back to BluetoothAdapter.getRemoteDevice for known addresses
- Extracts comprehensive device information (rssi, service UUIDs, bond state)
- Handles null scan records gracefully
780-989: LGTM! Comprehensive utility methods with proper error handling.The helper methods are well-implemented:
- Thread-safety via runOnMainThread
- Version-aware BLE operations (submitWrite for API 33+)
- Multiple encoding support (hex, utf8, base64)
- Robust UUID parsing with validation
- Proper exception handling throughout
990-1031: LGTM! Proper service discovery sequencing.The GATT callback correctly implements the service discovery flow per retrieved learnings:
- On connection (line 993-996): calls
discoverServices()but doesn't resolve the connect call yet- On services discovered (line 1005-1032): sets
servicesDiscovered = true, flushes pending notification calls, and then callsconnectedSuccessfully()at line 1031This ensures the 'open' event is only dispatched after GATT services are resolved, preventing early write failures.
Based on learnings: "CapacitorBluetooth.connect should only dispatch the 'open' event after GATT services are resolved and notifications are started; announcing open earlier causes the first MSP write to fail."
1038-1048: LGTM! Backward-compatible notification handling.The deprecated 2-arg
onCharacteristicChangedoverride properly:
- Retrieves value via
characteristic.getValue()for Android 12 and below- Includes null checks before delegation
- Forwards to the Android 13+ 3-arg version
- Annotated with
@Deprecatedand@SuppressWarnings("deprecation")This ensures notifications work across all Android versions (8-15 as tested).
1065-1114: LGTM! Comprehensive scan filtering logic.The
ScanCriteriaclass and itsmatches()method properly implement:
- Accept-all bypass for broad scanning
- Exact name matching
- Case-insensitive prefix matching
- Service UUID filtering (matches if any advertised UUID is in the filter list)
- Defaults to true when no service filter specified
The filtering logic correctly integrates with the scan flow at line 562.
224-234: Clarify redundancy between manual and Capacitor permission handling approaches.The plugin implements two distinct permission handling mechanisms:
- Manual approach via
requestPermissions()(lines 142-172) usingActivityCompat.requestPermissionsandhandleRequestPermissionsResult- Capacitor's
@PermissionCallback(lines 224-234) usinggetPermissionState("bluetooth")The manual approach correctly implements version-aware permissions (Android 12+ vs 8-11) with
Build.VERSION.SDK_INT >= Build.VERSION_CODES.Schecks at lines 178-189. However, it's unclear whether both mechanisms are active or if one is dead code. If both handle permission results when JS callsrequestPermissions(), they may conflict. Verify which code path is actually used and consolidate if redundant.
|
@coderabbitai the core reason the write function is slow is that it is performing expensive, time-consuming setup operations on every single write operation instead of performing them once after connection. A. We should add a class member to the BetaflightBluetoothPlugin.java class: private BluetoothGattCharacteristic writeCharacteristic = null; B. Initialize it Once on Connection The proper place to look up and cache this characteristic is inside the BluetoothGattCallback's onServicesDiscovered() method, which is called once after a successful connection and service discovery. C. Simplify the write Function - the new write function simply needs to check if the characteristic is available and then immediately use it to send data, eliminating all the slow lookups. Please analyze and propose a patch. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain@haslinghuis thanks — your plan is sound. While the script runs to confirm symbol names and exact locations, here’s the minimal, targeted patch I propose to implement your A/B/C:
Java patch (drop-in; I’ll adapt identifiers to match the file after the script output) // android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
// 1) Add fields near other member vars
private @Nullable BluetoothGatt bluetoothGatt = null; // if not already present
private @Nullable BluetoothGattCharacteristic writeCharacteristic = null;
private @Nullable BluetoothGattCharacteristic notifyCharacteristic = null;
private @Nullable UUID cachedServiceUuid = null;
private @Nullable UUID cachedWriteCharUuid = null;
private @Nullable UUID cachedNotifyCharUuid = null;
// 2) In your BluetoothGattCallback, inside onServicesDiscovered(...)
@Override
public void onServicesDiscovered(BluetoothGatt gatt, int status) {
if (status != BluetoothGatt.GATT_SUCCESS) {
// existing error handling...
return;
}
bluetoothGatt = gatt; // keep reference
writeCharacteristic = null;
notifyCharacteristic = null;
cachedServiceUuid = null;
cachedWriteCharUuid = null;
cachedNotifyCharUuid = null;
// If you already know the expected service/char UUIDs (from JS), keep using them.
// Otherwise, select the first service with a char that supports WRITE_NO_RESPONSE/WRITE.
for (BluetoothGattService svc : gatt.getServices()) {
BluetoothGattCharacteristic candidateWrite = null;
BluetoothGattCharacteristic candidateNotify = null;
for (BluetoothGattCharacteristic ch : svc.getCharacteristics()) {
final int props = ch.getProperties();
if (candidateWrite == null &&
((props & BluetoothGattCharacteristic.PROPERTY_WRITE_NO_RESPONSE) != 0 ||
(props & BluetoothGattCharacteristic.PROPERTY_WRITE) != 0)) {
candidateWrite = ch;
}
if (candidateNotify == null &&
(props & BluetoothGattCharacteristic.PROPERTY_NOTIFY) != 0) {
candidateNotify = ch;
}
if (candidateWrite != null && candidateNotify != null) break;
}
if (candidateWrite != null) {
writeCharacteristic = candidateWrite;
notifyCharacteristic = candidateNotify; // may be null, startNotifications() can still search if needed
cachedServiceUuid = svc.getUuid();
cachedWriteCharUuid = candidateWrite.getUuid();
if (candidateNotify != null) cachedNotifyCharUuid = candidateNotify.getUuid();
// Optionally: request MTU once here if you do that elsewhere
// gatt.requestMtu(247 or 517 depending on strategy)
break;
}
}
// existing code that emits "services" payload etc. can remain
// If you automatically start notifications, prefer the cached notifyCharacteristic there
}
// 3) In startNotifications(...), prefer the cached characteristic if available
private boolean startNotificationsInternal() {
if (bluetoothGatt == null) return false;
BluetoothGattCharacteristic ch = notifyCharacteristic;
if (ch == null) {
// fallback to your existing lookup if you must support dynamic target selection
// ch = findNotifyCharacteristicByKnownUuids(bluetoothGatt);
}
if (ch == null) return false;
boolean ok = bluetoothGatt.setCharacteristicNotification(ch, true);
if (!ok) return false;
// If you set CCCD descriptor, keep existing logic here
// BluetoothGattDescriptor cccd = ch.getDescriptor(CCCD_UUID);
// cccd.setValue(BluetoothGattDescriptor.ENABLE_NOTIFICATION_VALUE);
// bluetoothGatt.writeDescriptor(cccd);
return true;
}
// 4) Simplify write() to use the cached characteristic
@PluginMethod
public void write(PluginCall call) {
try {
if (bluetoothGatt == null || writeCharacteristic == null) {
call.reject("Not connected or services not discovered");
return;
}
// Decode input (base64/hex/utf8) to byte[]
byte[] value = decodePayload(call); // keep your existing decode logic
// Prefer WRITE_NO_RESPONSE when supported
final int props = writeCharacteristic.getProperties();
final int writeType = ((props & BluetoothGattCharacteristic.PROPERTY_WRITE_NO_RESPONSE) != 0)
? BluetoothGattCharacteristic.WRITE_TYPE_NO_RESPONSE
: BluetoothGattCharacteristic.WRITE_TYPE_DEFAULT;
int status;
if (android.os.Build.VERSION.SDK_INT >= android.os.Build.VERSION_CODES.TIRAMISU) {
status = bluetoothGatt.writeCharacteristic(writeCharacteristic, value, writeType);
} else {
writeCharacteristic.setWriteType(writeType);
writeCharacteristic.setValue(value);
// legacy returns boolean
boolean ok = bluetoothGatt.writeCharacteristic(writeCharacteristic);
status = ok ? android.bluetooth.BluetoothStatusCodes.SUCCESS
: android.bluetooth.BluetoothStatusCodes.ERROR_UNKNOWN;
}
if (status == android.bluetooth.BluetoothStatusCodes.SUCCESS) {
JSObject ret = new JSObject();
ret.put("bytesSent", value.length);
call.resolve(ret);
} else {
call.reject("GATT write failed: " + status);
}
} catch (Exception e) {
call.reject("Write error: " + e.getMessage(), e);
}
}
// 5) On disconnect/cleanup, null cached refs
private void cleanupGatt() {
if (bluetoothGatt != null) {
try { bluetoothGatt.disconnect(); } catch (Exception ignored) {}
try { bluetoothGatt.close(); } catch (Exception ignored) {}
}
bluetoothGatt = null;
writeCharacteristic = null;
notifyCharacteristic = null;
cachedServiceUuid = null;
cachedWriteCharUuid = null;
cachedNotifyCharUuid = null;
}Notes
If you post back the symbol names from the script (gatt field name, callback location), I’ll tailor the diff exactly to the current file to minimize churn. Would you like me to open a tiny follow-up PR with just this change so you can grab a CI artifact and measure the latency improvement on Android 8 and 15? 🏁 Script executed: Length of output: 20859 @haslinghuis great call. I inspected BetaflightBluetoothPlugin.java from your run and prepared a minimal, targeted patch that:
Patch (unified diff) diff --git a/android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java b/android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
index 0000000..0000001 100644
--- a/android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
+++ b/android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
@@ -86,6 +86,10 @@ public class BetaflightBluetoothPlugin extends Plugin {
private Runnable connectTimeoutRunnable;
private PluginCall pendingConnectCall;
private String connectedDeviceId;
+ // Cached, resolved once after service discovery to avoid per-write lookups
+ private volatile BluetoothGattCharacteristic writeCharacteristic = null;
+ // Optionally remember which service/char were selected (informational)
+ private volatile UUID cachedWriteServiceUuid = null, cachedWriteCharUuid = null;
private final Queue<PluginCall> pendingStartNotificationCalls = new ConcurrentLinkedQueue<>();
private volatile boolean servicesDiscovered = false;
@@ -1004,10 +1008,40 @@ public class BetaflightBluetoothPlugin extends Plugin {
@Override
public void onServicesDiscovered(BluetoothGatt gatt, int status) {
if (status != BluetoothGatt.GATT_SUCCESS) {
return;
}
servicesDiscovered = true;
+ // Resolve and cache a write characteristic exactly once per connection.
+ // Prefer WRITE_NO_RESPONSE, otherwise accept WRITE.
+ writeCharacteristic = null;
+ cachedWriteServiceUuid = null;
+ cachedWriteCharUuid = null;
+ try {
+ for (BluetoothGattService svc : gatt.getServices()) {
+ BluetoothGattCharacteristic preferred = null;
+ BluetoothGattCharacteristic fallback = null;
+ for (BluetoothGattCharacteristic ch : svc.getCharacteristics()) {
+ final int props = ch.getProperties();
+ if ((props & BluetoothGattCharacteristic.PROPERTY_WRITE_NO_RESPONSE) != 0) {
+ preferred = ch;
+ break; // best possible
+ } else if ((props & BluetoothGattCharacteristic.PROPERTY_WRITE) != 0) {
+ if (fallback == null) fallback = ch;
+ }
+ }
+ BluetoothGattCharacteristic chosen = (preferred != null) ? preferred : fallback;
+ if (chosen != null) {
+ writeCharacteristic = chosen;
+ cachedWriteServiceUuid = svc.getUuid();
+ cachedWriteCharUuid = chosen.getUuid();
+ break;
+ }
+ }
+ } catch (Exception ignored) {
+ // leave writeCharacteristic null; write() will fallback to per-call lookup
+ }
+
flushPendingStartNotificationCalls();
logGattLayout("Services discovered", gatt);
JSArray services = new JSArray();
for (BluetoothGattService service : gatt.getServices()) {
@@ -323,28 +357,79 @@ public class BetaflightBluetoothPlugin extends Plugin {
@PluginMethod
public void write(PluginCall call) {
if (!ensureConnected(call)) {
return;
}
- String serviceId = call.getString("service");
- String characteristicId = call.getString("characteristic");
- String value = call.getString("value", call.getString("data"));
- String encoding = call.getString("encoding", "base64");
- boolean withoutResponse = call.getBoolean("withoutResponse", false);
-
- if (TextUtils.isEmpty(serviceId) || TextUtils.isEmpty(characteristicId)) {
- call.reject("service and characteristic are required");
- return;
- }
+ final String value = call.getString("value", call.getString("data"));
+ final String encoding = call.getString("encoding", "base64");
+ final boolean withoutResponse = call.getBoolean("withoutResponse", false);
BluetoothGatt gatt = bluetoothGatt;
if (gatt == null) {
call.reject("Not connected");
return;
}
+ // Fast-path: use cached characteristic if available
+ BluetoothGattCharacteristic target = writeCharacteristic;
+ if (target != null) {
+ byte[] payload = decodeValue(value, encoding);
+ if (payload == null) {
+ call.reject("Invalid payload");
+ return;
+ }
+ final int props = target.getProperties();
+ final boolean canNoRsp = (props & BluetoothGattCharacteristic.PROPERTY_WRITE_NO_RESPONSE) != 0;
+ final int writeType = (withoutResponse || canNoRsp)
+ ? BluetoothGattCharacteristic.WRITE_TYPE_NO_RESPONSE
+ : BluetoothGattCharacteristic.WRITE_TYPE_DEFAULT;
+
+ int status;
+ if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) {
+ status = gatt.writeCharacteristic(target, payload, writeType);
+ } else {
+ target.setWriteType(writeType);
+ target.setValue(payload);
+ boolean ok = gatt.writeCharacteristic(target);
+ status = ok ? android.bluetooth.BluetoothStatusCodes.SUCCESS
+ : android.bluetooth.BluetoothStatusCodes.ERROR_UNKNOWN;
+ }
+
+ if (status == android.bluetooth.BluetoothStatusCodes.SUCCESS) {
+ JSObject result = new JSObject();
+ result.put("bytesSent", payload.length);
+ call.resolve(result);
+ } else {
+ call.reject("Unable to write characteristic");
+ }
+ return;
+ }
+
+ // Fallback (rare): no cached characteristic yet → use existing, explicit-UUID path.
+ String serviceId = call.getString("service");
+ String characteristicId = call.getString("characteristic");
+ if (TextUtils.isEmpty(serviceId) || TextUtils.isEmpty(characteristicId)) {
+ call.reject("service and characteristic are required");
+ return;
+ }
+
+ // Existing lookup/submit path (unchanged)
+ // parse UUIDs, locate service/characteristic, choose writeType, submitWrite(...)
+ // vvvvv keep current implementation from here vvvvv
String serviceId = call.getString("service");
String characteristicId = call.getString("characteristic");
String value = call.getString("value", call.getString("data"));
String encoding = call.getString("encoding", "base64");
boolean withoutResponse = call.getBoolean("withoutResponse", false);
if (TextUtils.isEmpty(serviceId) || TextUtils.isEmpty(characteristicId)) {
call.reject("service and characteristic are required");
return;
}
@@ -685,10 +770,16 @@ public class BetaflightBluetoothPlugin extends Plugin {
private void cleanupGatt() {
if (bluetoothGatt != null) {
try {
bluetoothGatt.disconnect();
} catch (Exception ignored) {}
try {
bluetoothGatt.close();
} catch (Exception ignored) {}
}
bluetoothGatt = null;
+ // Reset cached references so a fresh connection re-resolves them
+ writeCharacteristic = null;
+ cachedWriteServiceUuid = null;
+ cachedWriteCharUuid = null;
connectedDeviceId = null;
servicesDiscovered = false;
}What this does
Notes
Would you like me to open a tiny follow-up PR/commit on this branch with the above change so you can grab a CI artifact and re-test latency on Android 8 and 15? 🧠 Learnings used |
|
@coderabbitai - tried the code locally - but it does not build with errors, still want to have write responsible for writing only. IMO all other config should be done once on connect |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 15026 @haslinghuis got it — let’s make write() “write-only” and move all lookups to connect-time. Below is a minimal patch that compiles against your current file structure (keeps your submitWrite(...) helper and nulls cache on cleanup). Key points
Diff diff --git a/android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java b/android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
index 1111111..2222222 100644
--- a/android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
+++ b/android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
@@ -118,6 +118,11 @@ public class BetaflightBluetoothPlugin extends Plugin {
private Runnable connectTimeoutRunnable;
private PluginCall pendingConnectCall;
private String connectedDeviceId;
+ // Cached once after service discovery to avoid per-write lookups
+ private volatile BluetoothGattCharacteristic writeCharacteristic = null;
+ // Optional: for logging/inspection
+ private volatile UUID cachedWriteServiceUuid = null;
+ private volatile UUID cachedWriteCharUuid = null;
private final Queue<PluginCall> pendingStartNotificationCalls = new ConcurrentLinkedQueue<>();
private volatile boolean servicesDiscovered = false;
@@ -1005,10 +1010,39 @@ public class BetaflightBluetoothPlugin extends Plugin {
@Override
public void onServicesDiscovered(BluetoothGatt gatt, int status) {
if (status != BluetoothGatt.GATT_SUCCESS) {
return;
}
servicesDiscovered = true;
+ // Resolve and cache a write characteristic exactly once per connection.
+ // Prefer WRITE_NO_RESPONSE; otherwise accept WRITE.
+ try {
+ writeCharacteristic = null;
+ cachedWriteServiceUuid = null;
+ cachedWriteCharUuid = null;
+ for (BluetoothGattService svc : gatt.getServices()) {
+ BluetoothGattCharacteristic preferred = null;
+ BluetoothGattCharacteristic fallback = null;
+ for (BluetoothGattCharacteristic ch : svc.getCharacteristics()) {
+ final int props = ch.getProperties();
+ if ((props & BluetoothGattCharacteristic.PROPERTY_WRITE_NO_RESPONSE) != 0) {
+ preferred = ch;
+ break; // best option
+ } else if ((props & BluetoothGattCharacteristic.PROPERTY_WRITE) != 0) {
+ if (fallback == null) fallback = ch;
+ }
+ }
+ BluetoothGattCharacteristic chosen = (preferred != null) ? preferred : fallback;
+ if (chosen != null) {
+ writeCharacteristic = chosen;
+ cachedWriteServiceUuid = svc.getUuid();
+ cachedWriteCharUuid = chosen.getUuid();
+ break;
+ }
+ }
+ } catch (Exception e) {
+ Log.w(TAG, "Failed to cache write characteristic: " + e.getMessage());
+ }
+
flushPendingStartNotificationCalls();
logGattLayout("Services discovered", gatt);
JSArray services = new JSArray();
for (BluetoothGattService service : gatt.getServices()) {
JSObject servicePayload = new JSObject();
@@ -320,47 +354,61 @@ public class BetaflightBluetoothPlugin extends Plugin {
@PluginMethod
public void write(PluginCall call) {
- if (!ensureConnected(call)) {
- return;
- }
-
- String serviceId = call.getString("service");
- String characteristicId = call.getString("characteristic");
- String value = call.getString("value", call.getString("data"));
- String encoding = call.getString("encoding", "base64");
- boolean withoutResponse = call.getBoolean("withoutResponse", false);
-
- if (TextUtils.isEmpty(serviceId) || TextUtils.isEmpty(characteristicId)) {
- call.reject("service and characteristic are required");
- return;
- }
-
- BluetoothGatt gatt = bluetoothGatt;
- if (gatt == null) {
- call.reject("Not connected");
- return;
- }
+ // Write-only: rely on cached writeCharacteristic populated at service discovery time.
+ if (!ensureConnected(call)) return;
+ final BluetoothGatt gatt = bluetoothGatt;
+ if (!servicesDiscovered || gatt == null) { call.reject("Services not discovered"); return; }
+ final BluetoothGattCharacteristic target = writeCharacteristic;
+ if (target == null) { call.reject("Write characteristic not available"); return; }
+
+ final String value = call.getString("value", call.getString("data"));
+ final String encoding = call.getString("encoding", "base64");
+ final boolean withoutResponse = call.getBoolean("withoutResponse", false);
// Decode payload
- byte[] payload = decodeValue(value, encoding);
- if (payload == null) {
- call.reject("Invalid payload");
- return;
- }
+ byte[] payload;
+ try {
+ // reuse existing helpers if present; otherwise default to base64
+ // (keep this small to avoid dragging lookups into write)
+ if ("base64".equalsIgnoreCase(encoding) || TextUtils.isEmpty(encoding)) {
+ payload = (value != null) ? Base64.decode(value, Base64.DEFAULT) : null;
+ } else if ("utf8".equalsIgnoreCase(encoding)) {
+ payload = (value != null) ? value.getBytes(StandardCharsets.UTF_8) : null;
+ } else if ("hex".equalsIgnoreCase(encoding)) {
+ payload = hexToBytes(value); // uses existing helper if defined below
+ } else {
+ payload = Base64.decode(value, Base64.DEFAULT);
+ }
+ } catch (Exception e) {
+ payload = null;
+ }
+ if (payload == null) { call.reject("Invalid payload"); return; }
// Choose write type: prefer NO_RESPONSE when either requested or supported
- int writeType = withoutResponse
- ? BluetoothGattCharacteristic.WRITE_TYPE_NO_RESPONSE
- : BluetoothGattCharacteristic.WRITE_TYPE_DEFAULT;
+ final int props = target.getProperties();
+ final boolean canNoRsp = (props & BluetoothGattCharacteristic.PROPERTY_WRITE_NO_RESPONSE) != 0;
+ final int writeType = (withoutResponse || canNoRsp)
+ ? BluetoothGattCharacteristic.WRITE_TYPE_NO_RESPONSE
+ : BluetoothGattCharacteristic.WRITE_TYPE_DEFAULT;
- // Find the characteristic and submit the write
- UUID serviceUuid = parseUuid(serviceId);
- UUID characteristicUuid = parseUuid(characteristicId);
- BluetoothGattService service = gatt.getService(serviceUuid);
- if (service == null) { call.reject("Service not found: " + serviceId); return; }
- BluetoothGattCharacteristic characteristic = service.getCharacteristic(characteristicUuid);
- if (characteristic == null) { call.reject("Characteristic not found: " + characteristicId); return; }
- boolean ok = submitWrite(gatt, characteristic, payload, writeType);
+ boolean ok = submitWrite(gatt, target, payload, writeType);
if (ok) {
JSObject result = new JSObject();
result.put("bytesSent", payload.length);
call.resolve(result);
} else {
- call.reject("Unable to write characteristic");
+ call.reject("Unable to write characteristic");
}
}
@@ -680,10 +728,16 @@ public class BetaflightBluetoothPlugin extends Plugin {
private void cleanupGatt() {
if (bluetoothGatt != null) {
try { bluetoothGatt.disconnect(); } catch (Exception ignored) {}
try { bluetoothGatt.close(); } catch (Exception ignored) {}
}
bluetoothGatt = null;
+ // reset cached references
+ writeCharacteristic = null;
+ cachedWriteServiceUuid = null;
+ cachedWriteCharUuid = null;
connectedDeviceId = null;
servicesDiscovered = false;
}
+ // If not already present, provide a tiny hex decoder used above
+ private static byte[] hexToBytes(String hex) {
+ if (hex == null) return null;
+ int len = hex.length();
+ if ((len & 1) != 0) return null;
+ byte[] data = new byte[len / 2];
+ for (int i = 0; i < len; i += 2) {
+ int hi = Character.digit(hex.charAt(i), 16);
+ int lo = Character.digit(hex.charAt(i + 1), 16);
+ if (hi < 0 || lo < 0) return null;
+ data[i / 2] = (byte) ((hi << 4) + lo);
+ }
+ return data;
+ }Why this matches your ask
If the build still errors
Do you want me to follow up with a tiny CI build PR with just this change so you can measure write latency on Android 8 and 15? 🧠 Learnings used |
|
@coderabbitai - please do not add other things - like hex decoder. Also the proposed code does not resemble the last commit in this PR. |
|
Tip For best results, initiate chat on the files or code changes. @haslinghuis got it — no extra helpers and stick to the current structure. Here’s a minimal patch that only:
Patch (unified diff; minimal edits only) diff --git a/android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java b/android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
index XXXXXXX..YYYYYYY 100644
--- a/android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
+++ b/android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
@@ -120,6 +120,8 @@ public class BetaflightBluetoothPlugin extends Plugin {
private Runnable connectTimeoutRunnable;
private PluginCall pendingConnectCall;
private String connectedDeviceId;
+ // Cached once after service discovery; used by write()
+ private BluetoothGattCharacteristic writeCharacteristic = null;
private final Queue<PluginCall> pendingStartNotificationCalls = new ConcurrentLinkedQueue<>();
private volatile boolean servicesDiscovered = false;
@@ -1008,6 +1010,26 @@ public class BetaflightBluetoothPlugin extends Plugin {
@Override
public void onServicesDiscovered(BluetoothGatt gatt, int status) {
if (status != BluetoothGatt.GATT_SUCCESS) {
return;
}
servicesDiscovered = true;
+ // Resolve and cache a write characteristic once per connection.
+ // Prefer WRITE_NO_RESPONSE; otherwise accept WRITE.
+ writeCharacteristic = null;
+ try {
+ for (BluetoothGattService svc : gatt.getServices()) {
+ BluetoothGattCharacteristic preferred = null;
+ BluetoothGattCharacteristic fallback = null;
+ for (BluetoothGattCharacteristic ch : svc.getCharacteristics()) {
+ final int props = ch.getProperties();
+ if ((props & BluetoothGattCharacteristic.PROPERTY_WRITE_NO_RESPONSE) != 0) { preferred = ch; break; }
+ if (fallback == null && (props & BluetoothGattCharacteristic.PROPERTY_WRITE) != 0) { fallback = ch; }
+ }
+ BluetoothGattCharacteristic chosen = (preferred != null) ? preferred : fallback;
+ if (chosen != null) { writeCharacteristic = chosen; break; }
+ }
+ } catch (Exception ignored) {
+ // leave writeCharacteristic null; write() will report unavailable
+ }
flushPendingStartNotificationCalls();
// ... existing services payload emission remains unchanged ...
@@ -330,39 +352,28 @@ public class BetaflightBluetoothPlugin extends Plugin {
@PluginMethod
public void write(PluginCall call) {
- if (!ensureConnected(call)) {
- return;
- }
-
- String serviceId = call.getString("service");
- String characteristicId = call.getString("characteristic");
- String value = call.getString("value", call.getString("data"));
- String encoding = call.getString("encoding", "base64");
- boolean withoutResponse = call.getBoolean("withoutResponse", false);
-
- if (TextUtils.isEmpty(serviceId) || TextUtils.isEmpty(characteristicId)) {
- call.reject("service and characteristic are required");
- return;
- }
-
- BluetoothGatt gatt = bluetoothGatt;
- if (gatt == null) {
- call.reject("Not connected");
- return;
- }
+ if (!ensureConnected(call)) return;
+ final BluetoothGatt gatt = bluetoothGatt;
+ if (gatt == null || !servicesDiscovered) { call.reject("Not connected"); return; }
+ final BluetoothGattCharacteristic target = writeCharacteristic;
+ if (target == null) { call.reject("Write characteristic not available"); return; }
- // Decode payload and choose write type; then submit
- UUID serviceUuid = parseUuid(serviceId);
- UUID characteristicUuid = parseUuid(characteristicId);
- BluetoothGattService service = gatt.getService(serviceUuid);
- if (service == null) { call.reject("Service not found: " + serviceId); return; }
- BluetoothGattCharacteristic characteristic = service.getCharacteristic(characteristicUuid);
- if (characteristic == null) { call.reject("Characteristic not found: " + characteristicId); return; }
+ final String value = call.getString("value", call.getString("data"));
+ final String encoding = call.getString("encoding", "base64");
+ final boolean withoutResponse = call.getBoolean("withoutResponse", false);
+ byte[] payload = decodeValue(value, encoding);
+ if (payload == null) { call.reject("Invalid payload"); return; }
- int writeType = withoutResponse
- ? BluetoothGattCharacteristic.WRITE_TYPE_NO_RESPONSE
- : BluetoothGattCharacteristic.WRITE_TYPE_DEFAULT;
- boolean ok = submitWrite(gatt, characteristic, payload, writeType);
+ final int props = target.getProperties();
+ final boolean canNoRsp = (props & BluetoothGattCharacteristic.PROPERTY_WRITE_NO_RESPONSE) != 0;
+ final int writeType = (withoutResponse || canNoRsp)
+ ? BluetoothGattCharacteristic.WRITE_TYPE_NO_RESPONSE
+ : BluetoothGattCharacteristic.WRITE_TYPE_DEFAULT;
+ boolean ok = submitWrite(gatt, target, payload, writeType);
if (ok) {
JSObject result = new JSObject();
result.put("bytesSent", payload.length);
call.resolve(result);
} else {
call.reject("Unable to write characteristic");
}
}
@@ -690,6 +701,7 @@ public class BetaflightBluetoothPlugin extends Plugin {
private void cleanupGatt() {
if (bluetoothGatt != null) {
try { bluetoothGatt.disconnect(); } catch (Exception ignored) {}
try { bluetoothGatt.close(); } catch (Exception ignored) {}
}
bluetoothGatt = null;
+ writeCharacteristic = null;
connectedDeviceId = null;
servicesDiscovered = false;
}Notes
If you prefer, I can adapt this to preserve the existing service/characteristic parameters in write() purely for validation/logging, but the fast path will still use the cached target. Want me to open a tiny follow-up patch PR with exactly this change so you can grab a CI artifact and measure? 🧠 Learnings used |
|
@coderabbitai - sorry but still your suggested patch is not using the last commit in this PR - and again results in build failures. This is how write functions looks like now: |
|
Tip For best results, initiate chat on the files or code changes. @haslinghuis thanks for the concrete snippet — I see why the build is failing and why the write path still feels heavy. Two issues in the current write():
Below is a minimal patch that compiles against your latest structure, keeps write() “write-only”, and moves all characteristic resolution to onServicesDiscovered(). It reuses your existing decodePayload(...) and submitWrite(...).
// Class field (near other fields)
private BluetoothGattCharacteristic writeCharacteristic = null;
@Override
public void onServicesDiscovered(BluetoothGatt gatt, int status) {
if (status != BluetoothGatt.GATT_SUCCESS) return;
servicesDiscovered = true;
// Resolve and cache a write characteristic once per connection
writeCharacteristic = null;
try {
for (BluetoothGattService svc : gatt.getServices()) {
BluetoothGattCharacteristic preferred = null;
BluetoothGattCharacteristic fallback = null;
for (BluetoothGattCharacteristic ch : svc.getCharacteristics()) {
final int props = ch.getProperties();
if ((props & BluetoothGattCharacteristic.PROPERTY_WRITE_NO_RESPONSE) != 0) {
preferred = ch; // best option for MSP
break;
}
if (fallback == null && (props & BluetoothGattCharacteristic.PROPERTY_WRITE) != 0) {
fallback = ch;
}
}
final BluetoothGattCharacteristic chosen = (preferred != null) ? preferred : fallback;
if (chosen != null) {
writeCharacteristic = chosen;
break;
}
}
} catch (Exception ignored) {
// leave writeCharacteristic null; write() will report unavailable
}
// ... keep your existing code (flushPendingStartNotificationCalls, emit services, etc.)
}
@PluginMethod
public void write(PluginCall call) {
if (!ensureConnected(call)) return;
final BluetoothGatt gatt = bluetoothGatt;
if (gatt == null || !servicesDiscovered) {
call.reject("Not connected");
return;
}
final BluetoothGattCharacteristic target = writeCharacteristic;
if (target == null) {
call.reject("Write characteristic not available");
return;
}
// Fetch payload params from call (they were previously undefined in this method)
final String value = call.getString("value", call.getString("data"));
final String encoding = call.getString("encoding", "base64");
final boolean withoutResponse = call.getBoolean("withoutResponse", false);
// Decode payload using your existing helper
final byte[] payload;
try {
payload = decodePayload(value, encoding);
} catch (IllegalArgumentException ex) {
call.reject("Failed to decode payload: " + ex.getMessage());
return;
}
// Choose write type: prefer NO_RESPONSE when supported or requested
final int props = target.getProperties();
final boolean canNoRsp = (props & BluetoothGattCharacteristic.PROPERTY_WRITE_NO_RESPONSE) != 0;
final int writeType = (withoutResponse || canNoRsp)
? BluetoothGattCharacteristic.WRITE_TYPE_NO_RESPONSE
: BluetoothGattCharacteristic.WRITE_TYPE_DEFAULT;
final boolean ok = submitWrite(gatt, target, payload, writeType);
if (ok) {
JSObject result = new JSObject();
result.put("bytesSent", payload.length);
call.resolve(result);
} else {
call.reject("Unable to write characteristic");
}
}
private void cleanupGatt() {
if (bluetoothGatt != null) {
try { bluetoothGatt.disconnect(); } catch (Exception ignored) {}
try { bluetoothGatt.close(); } catch (Exception ignored) {}
}
bluetoothGatt = null;
writeCharacteristic = null; // reset for next connection
connectedDeviceId = null;
servicesDiscovered = false;
}What this fixes
If you want to keep accepting service/characteristic in the API for compatibility, we can log-validate them against the cached target’s UUIDs, but not use them for lookup — let me know if you want that small addition. I can turn this into a tiny follow-up commit in your branch if that helps you grab a CI artifact quickly. ✏️ Learnings added
🧠 Learnings used |
|
@coderabbitai - perhaps also remove writeType out of write - this function really should be minimal for performance. |
|
Tip For best results, initiate chat on the files or code changes. @haslinghuis great idea — here’s a minimal patch that removes writeType logic from write() and moves the decision to connect-time. No new helpers added, and it keeps your current decodePayload(...) and submitWrite(...).
// Cached once per connection
private BluetoothGattCharacteristic writeCharacteristic = null;
private boolean writeNoResponseSupported = false; // computed once on discovery
@Override
public void onServicesDiscovered(BluetoothGatt gatt, int status) {
if (status != BluetoothGatt.GATT_SUCCESS) return;
servicesDiscovered = true;
writeCharacteristic = null;
writeNoResponseSupported = false;
try {
for (BluetoothGattService svc : gatt.getServices()) {
BluetoothGattCharacteristic preferred = null;
BluetoothGattCharacteristic fallback = null;
for (BluetoothGattCharacteristic ch : svc.getCharacteristics()) {
final int props = ch.getProperties();
if ((props & BluetoothGattCharacteristic.PROPERTY_WRITE_NO_RESPONSE) != 0) {
preferred = ch; // best option
break;
}
if (fallback == null && (props & BluetoothGattCharacteristic.PROPERTY_WRITE) != 0) {
fallback = ch;
}
}
final BluetoothGattCharacteristic chosen = (preferred != null) ? preferred : fallback;
if (chosen != null) {
writeCharacteristic = chosen;
writeNoResponseSupported =
(chosen.getProperties() & BluetoothGattCharacteristic.PROPERTY_WRITE_NO_RESPONSE) != 0;
break;
}
}
} catch (Exception ignored) {
// leave caches as-is; write() will reject if unavailable
}
// ... existing code (flushPendingStartNotificationCalls, emit services, etc.) ...
}
@PluginMethod
public void write(PluginCall call) {
if (!ensureConnected(call)) return;
final BluetoothGatt gatt = bluetoothGatt;
if (gatt == null || !servicesDiscovered) { call.reject("Not connected"); return; }
final BluetoothGattCharacteristic target = writeCharacteristic;
if (target == null) { call.reject("Write characteristic not available"); return; }
// Params and payload decode only
final String value = call.getString("value", call.getString("data"));
final String encoding = call.getString("encoding", "base64");
final boolean withoutResponse = call.getBoolean("withoutResponse", false);
final byte[] payload;
try {
payload = decodePayload(value, encoding);
} catch (IllegalArgumentException ex) {
call.reject("Failed to decode payload: " + ex.getMessage());
return;
}
// Minimal branch based on cached capability (no property reads here)
final int writeType = (withoutResponse && writeNoResponseSupported)
? BluetoothGattCharacteristic.WRITE_TYPE_NO_RESPONSE
: BluetoothGattCharacteristic.WRITE_TYPE_DEFAULT;
final boolean ok = submitWrite(gatt, target, payload, writeType);
if (ok) {
JSObject result = new JSObject();
result.put("bytesSent", payload.length);
call.resolve(result);
} else {
call.reject("Unable to write characteristic");
}
}
private void cleanupGatt() {
if (bluetoothGatt != null) {
try { bluetoothGatt.disconnect(); } catch (Exception ignored) {}
try { bluetoothGatt.close(); } catch (Exception ignored) {}
}
bluetoothGatt = null;
writeCharacteristic = null;
writeNoResponseSupported = false;
connectedDeviceId = null;
servicesDiscovered = false;
}Notes
If you’d like, I can adapt it to the “single cached writeType” variant as well. 🧠 Learnings used |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java (1)
61-69: Capacitor permission alias is effectively always GRANTED;ensureBluetoothReady()doesn’t enforce OS Bluetooth runtime permsYou define a Capacitor permission alias with no Android permission strings:
@CapacitorPlugin( name = "BetaflightBluetooth", permissions = { @Permission( alias = "bluetooth", strings = {} ) } )and then gate operations with:
if (getPermissionState("bluetooth") != PermissionState.GRANTED) { call.reject("Bluetooth permission not granted"); return false; }But an alias with an empty
stringsarray is treated by Capacitor as always granted on Android, soensureBluetoothReady()will never fail based on real OS Bluetooth/Location permissions. The actual runtime permission flow is handled separately viaActivityCompat.requestPermissionsandhandleRequestPermissionsResult, which is fine, but it means the Capacitor-level permission state and the OS-level state can diverge.To make this clearer and less surprising, I’d recommend one of:
- Wire the alias to the real Android permission strings and use Capacitor’s
requestPermissionForAlias/onPermissionResultcallback instead of the custom ActivityCompat path, sogetPermissionState("bluetooth")truly reflects OS state; or- If you prefer the manual ActivityCompat approach, drop the alias +
getPermissionState("bluetooth")check and rely on your explicit runtime permission checks only, to avoid a no-op permission gate.Based on learnings, this plugin will only run on Android, so aligning the alias with actual Android permissions is likely the cleanest long-term approach. Please double‑check current Capacitor permission semantics against your target Capacitor version.
Also applies to: 143-178, 179-192, 226-236, 662-676
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java(1 hunks)
🧰 Additional context used
🧠 Learnings (17)
📓 Common learnings
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-28T22:51:11.691Z
Learning: In betaflight-configurator, Web Bluetooth API's `services` option filters scan results to only devices advertising those UUIDs, while `optionalServices` requests permission to access services after connection without filtering. PWA (WebBluetooth.js) uses `{ acceptAllDevices: true, optionalServices: uuids }` to show all devices while requesting post-connection service access. CapacitorBluetooth.js should match this pattern rather than using `services` as a scan filter.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-02T17:46:47.828Z
Learning: BetaflightBluetoothPlugin.java: maintainers prefer write(PluginCall) to be “write-only” — no per-call UUID parsing or characteristic lookups. Resolve and cache the write characteristic once in onServicesDiscovered() (prefer PROPERTY_WRITE_NO_RESPONSE; else PROPERTY_WRITE). write() should read payload params from call, use the cached writeCharacteristic, pick write type, and submitWrite; clear the cache in cleanupGatt().
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-01T17:09:13.412Z
Learning: In betaflight-configurator’s CapacitorBluetooth.js, native plugin listeners (notification, services, connectionState) must be attached exactly once per app lifetime. Re-attaching them during writes/connect cycles leads to duplicate handlers, increased JS workload, MSP queue pressure, and can prevent the MSP_EEPROM_WRITE callback from firing (so reinitializeConnection is not reached). Implement an idempotent _attachNativeListenersOnce() guard.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-01T17:09:13.412Z
Learning: CapacitorBluetooth.connect should only dispatch the 'open' event after GATT services are resolved and notifications are started; announcing open earlier causes the first MSP write to fail with “Unable to write characteristic” and contributes to early queue load.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:135-143
Timestamp: 2025-11-28T22:41:59.374Z
Learning: In betaflight-configurator, WebSerial.js uses a constant path "serial" for all devices, and CapacitorBluetooth.js uses constant path "bluetooth", consistent with the single-device-at-a-time workflow. WebBluetooth.js uses counter-based paths (bluetooth_${counter}), but that's specific to its implementation. The path strategy varies by protocol based on their specific requirements.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-30T18:10:26.784Z
Learning: In betaflight-configurator's Android Bluetooth implementation, permission handling must be version-aware: Android 8-11 (API 26-30) requires BLUETOOTH, BLUETOOTH_ADMIN, and ACCESS_COARSE_LOCATION permissions, while Android 12+ (API 31+) requires BLUETOOTH_SCAN and BLUETOOTH_CONNECT. The AndroidManifest.xml should use android:maxSdkVersion="30" on legacy permissions and android:minSdkVersion="31" on new permissions. The Java code must conditionally request permissions using Build.VERSION.SDK_INT >= Build.VERSION_CODES.S check to avoid requesting non-existent permissions on older Android versions.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-01T17:38:41.581Z
Learning: On BLE (Android) Save & Reboot, status pull intervals continue during MSP_EEPROM_WRITE, overwhelming the MSP queue and causing the EEPROM ack to time out so reinitializeConnection never runs. Solution: before EEPROM write, call GUI.interval_kill_all() and MSP.callbacks_cleanup(), and add a 3s fallback to proceed to reboot if the ack is missed.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-06-09T00:32:21.385Z
Learning: In the betaflight-configurator codebase, port paths use counter prefixes (e.g., "bluetooth1", "bluetooth2", "serial1") rather than direct protocol identifiers. The protocol selection logic correctly uses `portPath.startsWith("bluetooth")` to detect bluetooth ports regardless of the counter suffix, rather than direct string matching against protocol map keys.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4706
File: src/js/protocols/CapacitorSerial.js:47-47
Timestamp: 2025-11-24T15:07:25.227Z
Learning: In betaflight-configurator's CapacitorSerial protocol (src/js/protocols/CapacitorSerial.js), the fire-and-forget this.loadDevices() call in the constructor is intentional background pre-loading. The actual critical device list loading for UI population is orchestrated by port_handler.js via serial.getDevices("serial"), which properly awaits CapacitorSerial.getDevices() → loadDevices() in a complete async chain. The constructor's async call does not cause race conditions.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4510
File: src/js/msp.js:384-391
Timestamp: 2025-09-19T20:41:44.286Z
Learning: When fixing MSP duplicate handling in Betaflight Configurator, avoid complex changes to callback resolution mechanisms as they can break tab switching functionality. Simple duplicate detection based on code and payload size is safer than complex requestKey-based approaches.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:51:09.253Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported for BLE devices and Web USB API is supported, but Web Serial API is not supported (except limited Bluetooth serial support added in 2025). The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:39:16.388Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported from version 133+ (requires Android 6.0+) and Web USB API is supported from version 136+, but Web Serial API is not supported. The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.
📚 Learning: 2025-12-02T17:46:47.828Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-02T17:46:47.828Z
Learning: BetaflightBluetoothPlugin.java: maintainers prefer write(PluginCall) to be “write-only” — no per-call UUID parsing or characteristic lookups. Resolve and cache the write characteristic once in onServicesDiscovered() (prefer PROPERTY_WRITE_NO_RESPONSE; else PROPERTY_WRITE). write() should read payload params from call, use the cached writeCharacteristic, pick write type, and submitWrite; clear the cache in cleanupGatt().
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-11-30T18:10:26.784Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-30T18:10:26.784Z
Learning: In betaflight-configurator's Android Bluetooth implementation, permission handling must be version-aware: Android 8-11 (API 26-30) requires BLUETOOTH, BLUETOOTH_ADMIN, and ACCESS_COARSE_LOCATION permissions, while Android 12+ (API 31+) requires BLUETOOTH_SCAN and BLUETOOTH_CONNECT. The AndroidManifest.xml should use android:maxSdkVersion="30" on legacy permissions and android:minSdkVersion="31" on new permissions. The Java code must conditionally request permissions using Build.VERSION.SDK_INT >= Build.VERSION_CODES.S check to avoid requesting non-existent permissions on older Android versions.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-11-28T22:51:11.691Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-28T22:51:11.691Z
Learning: In betaflight-configurator, Web Bluetooth API's `services` option filters scan results to only devices advertising those UUIDs, while `optionalServices` requests permission to access services after connection without filtering. PWA (WebBluetooth.js) uses `{ acceptAllDevices: true, optionalServices: uuids }` to show all devices while requesting post-connection service access. CapacitorBluetooth.js should match this pattern rather than using `services` as a scan filter.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-12-01T17:09:13.412Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-01T17:09:13.412Z
Learning: In betaflight-configurator’s CapacitorBluetooth.js, native plugin listeners (notification, services, connectionState) must be attached exactly once per app lifetime. Re-attaching them during writes/connect cycles leads to duplicate handlers, increased JS workload, MSP queue pressure, and can prevent the MSP_EEPROM_WRITE callback from firing (so reinitializeConnection is not reached). Implement an idempotent _attachNativeListenersOnce() guard.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-06-09T00:32:21.385Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-06-09T00:32:21.385Z
Learning: In the betaflight-configurator codebase, port paths use counter prefixes (e.g., "bluetooth1", "bluetooth2", "serial1") rather than direct protocol identifiers. The protocol selection logic correctly uses `portPath.startsWith("bluetooth")` to detect bluetooth ports regardless of the counter suffix, rather than direct string matching against protocol map keys.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-05-14T21:51:09.253Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:51:09.253Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported for BLE devices and Web USB API is supported, but Web Serial API is not supported (except limited Bluetooth serial support added in 2025). The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-11-28T22:41:59.374Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:135-143
Timestamp: 2025-11-28T22:41:59.374Z
Learning: In betaflight-configurator, WebSerial.js uses a constant path "serial" for all devices, and CapacitorBluetooth.js uses constant path "bluetooth", consistent with the single-device-at-a-time workflow. WebBluetooth.js uses counter-based paths (bluetooth_${counter}), but that's specific to its implementation. The path strategy varies by protocol based on their specific requirements.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-09-19T20:41:44.286Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4510
File: src/js/msp.js:384-391
Timestamp: 2025-09-19T20:41:44.286Z
Learning: When fixing MSP duplicate handling in Betaflight Configurator, avoid complex changes to callback resolution mechanisms as they can break tab switching functionality. Simple duplicate detection based on code and payload size is safer than complex requestKey-based approaches.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-09-19T20:42:20.332Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4510
File: src/js/msp.js:384-391
Timestamp: 2025-09-19T20:42:20.332Z
Learning: Complex MSP duplicate handling fixes in Betaflight Configurator can cause infinite loading messages when changing tabs due to disruption of the callback resolution mechanism. Simple code-only duplicate detection (using this.callbacks.some((instance) => instance.code === code)) is the safer approach that preserves tab switching functionality.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-09-09T20:02:33.475Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4597
File: src/js/utils/common.js:95-127
Timestamp: 2025-09-09T20:02:33.475Z
Learning: In the Betaflight Configurator codebase, the Chromium v140 compatibility issue with sortSelect was resolved by rewriting the implementation to use native DOM APIs (Array.from, select.options, select.remove, select.add) instead of jQuery DOM manipulation methods (this.children, this.empty().append). The fix still performs DOM operations but avoids the specific jQuery methods that were causing issues in Chromium v140.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-06-27T22:06:49.210Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4532
File: src/js/VirtualFC.js:234-234
Timestamp: 2025-06-27T22:06:49.210Z
Learning: In the betaflight-configurator codebase, the VirtualFC.js AUX_CONFIG array must stay synchronized with the betaflight firmware's msp_box.c definitions to ensure proper auxiliary mode functionality. Changes to mode names should follow the firmware source code even if it potentially breaks backward compatibility with saved presets.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-08-27T22:07:26.353Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4579
File: src/js/data_storage.js:9-9
Timestamp: 2025-08-27T22:07:26.353Z
Learning: In the betaflight-configurator CalVer refactoring, the generateVirtualApiVersions function in src/js/utils/common.js works correctly despite the apparent mathematical issue with bounds logic where compareVersions.minor() returns different ranges for API_VERSION_MAX_SUPPORTED (25.12.0) vs API_VERSION_ACCEPTED (1.44.0).
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-08-11T19:10:56.992Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4567
File: locales/en/messages.json:7724-7728
Timestamp: 2025-08-11T19:10:56.992Z
Learning: In the Betaflight Configurator project, only English language entries are added directly to locales/en/messages.json. Translations for other languages are handled through an outsourced process and should not be suggested as part of code reviews.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-09-10T18:26:10.136Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4601
File: package.json:125-125
Timestamp: 2025-09-10T18:26:10.136Z
Learning: In betaflight-configurator, dependency updates are handled incrementally - Vue dependencies are updated separately from Vite dependencies for better isolation and maintainability.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-11-19T17:55:51.714Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4708
File: src/js/tabs/firmware_flasher.js:0-0
Timestamp: 2025-11-19T17:55:51.714Z
Learning: In the betaflight-configurator project, only current Chrome and Edge-based browsers are officially supported, and users are expected to keep their browsers up-to-date for security reasons. This means ES2023 features like Array.prototype.toSorted() and Object.groupBy() can be safely used without polyfills or transpilation.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-12-01T17:09:13.412Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-01T17:09:13.412Z
Learning: CapacitorBluetooth.connect should only dispatch the 'open' event after GATT services are resolved and notifications are started; announcing open earlier causes the first MSP write to fail with “Unable to write characteristic” and contributes to early queue load.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: deploy
|
@coderabbitai - lag in write has improved as the model in setup tab follows movement - but somehow the reboot issue - settings are not updated - still remains. Please analyze as we still do not reach reinitializeConnection in serial_backend.js |
|
Tip For best results, initiate chat on the files or code changes. @haslinghuis thanks for the update. Great to hear write latency is fixed. The remaining “Save & Reboot doesn’t reach reinitializeConnection” is consistent with the MSP_EEPROM_WRITE callback occasionally being starved on BLE, even after stopping intervals/cleaning callbacks. The simplest, low‑risk fix is to add a bounded fallback: if the EEPROM ack doesn’t arrive within a few seconds, still proceed to the reboot flow. Below is a minimal change to MSPHelper.writeConfiguration that keeps your current behavior (GUI.interval_kill_all(); MSP.callbacks_cleanup(); + small delay) and adds a 3s fallback to ensure reinitializeConnection is reached regardless of BLE timing. No extra helpers, no structural changes. Patch: src/js/msp/MSPHelper.js (writeConfiguration) // …existing code above…
MspHelper.prototype.writeConfiguration = function (reboot, callback) {
// Quiet background traffic to avoid queue starvation on BLE
try {
GUI.interval_kill_all();
MSP.callbacks_cleanup();
} catch (e) {
console.warn("writeConfiguration: pre-save quieting failed:", e);
}
const sendEeprom = () => {
let finished = false;
const finish = () => {
if (finished) return;
finished = true;
if (reboot) {
GUI.tab_switch_cleanup(function () {
return reinitializeConnection();
});
}
if (callback) callback();
};
// Fallback in case the EEPROM ack is missed under BLE load
const fallbackTimer = setTimeout(() => {
console.warn("MSP_EEPROM_WRITE ack timeout; proceeding to reboot via fallback.");
finish();
}, 3000); // conservative for Android BLE
MSP.send_message(MSPCodes.MSP_EEPROM_WRITE, false, false, function () {
clearTimeout(fallbackTimer);
gui_log(i18n.getMessage("configurationEepromSaved"));
console.log("Configuration saved to EEPROM");
finish();
});
};
// Keep your arming safety, but don’t block on it
if (!FC.CONFIG.armingDisabled) {
this.setArmingEnabled(false, false);
setTimeout(sendEeprom, 200); // current delay retained
} else {
setTimeout(sendEeprom, 200);
}
};
// …existing code below…Why this solves it
Quick validation
If you want me to fold this into the branch as a tiny follow-up commit for a CI artifact, say the word and I’ll prep it. I’ve kept this aligned with the PR’s current structure and avoided adding any new utilities. 🧠 Learnings used |
|
@coderabbitai - applied the code - not submitted yet - on reboot getting |
|
Tip For best results, initiate chat on the files or code changes. @haslinghuis thanks — that error typically means we’re trying to write during the brief window after reconnect (reboot) before GATT services are rediscovered and the cached writeCharacteristic is valid, or we’re still holding a stale characteristic from the previous GATT. Two minimal, surgical fixes that keep write() write‑only and align with your last commit:
Java (BetaflightBluetoothPlugin.java)
// Cached per connection
private BluetoothGattCharacteristic writeCharacteristic = null;
private boolean writeNoResponseSupported = false;
private int cachedWriteType = BluetoothGattCharacteristic.WRITE_TYPE_DEFAULT;
writeCharacteristic = null;
writeNoResponseSupported = false;
cachedWriteType = BluetoothGattCharacteristic.WRITE_TYPE_DEFAULT;
try {
for (BluetoothGattService svc : gatt.getServices()) {
BluetoothGattCharacteristic preferred = null, fallback = null;
for (BluetoothGattCharacteristic ch : svc.getCharacteristics()) {
int props = ch.getProperties();
if ((props & BluetoothGattCharacteristic.PROPERTY_WRITE_NO_RESPONSE) != 0) { preferred = ch; break; }
if (fallback == null && (props & BluetoothGattCharacteristic.PROPERTY_WRITE) != 0) fallback = ch;
}
BluetoothGattCharacteristic chosen = (preferred != null) ? preferred : fallback;
if (chosen != null) {
writeCharacteristic = chosen;
writeNoResponseSupported =
(chosen.getProperties() & BluetoothGattCharacteristic.PROPERTY_WRITE_NO_RESPONSE) != 0;
cachedWriteType = writeNoResponseSupported
? BluetoothGattCharacteristic.WRITE_TYPE_NO_RESPONSE
: BluetoothGattCharacteristic.WRITE_TYPE_DEFAULT;
break;
}
}
} catch (Exception ignored) {
// leave writeCharacteristic null; write() will reject
}
@PluginMethod
public void write(PluginCall call) {
if (!ensureConnected(call)) return;
final BluetoothGatt gatt = bluetoothGatt;
if (gatt == null || !servicesDiscovered) { call.reject("Not connected"); return; }
final BluetoothGattCharacteristic target = writeCharacteristic;
if (target == null) { call.reject("Write characteristic not available"); return; }
final String value = call.getString("value", call.getString("data"));
final String encoding = call.getString("encoding", "base64");
final byte[] payload;
try { payload = decodePayload(value, encoding); }
catch (IllegalArgumentException ex) { call.reject("Failed to decode payload: " + ex.getMessage()); return; }
final boolean ok = submitWrite(gatt, target, payload, cachedWriteType);
if (ok) {
JSObject result = new JSObject();
result.put("bytesSent", payload.length);
call.resolve(result);
} else {
call.reject("Unable to write characteristic");
}
}
private void cleanupGatt() {
if (bluetoothGatt != null) {
try { bluetoothGatt.disconnect(); } catch (Exception ignored) {}
try { bluetoothGatt.close(); } catch (Exception ignored) {}
}
bluetoothGatt = null;
writeCharacteristic = null;
writeNoResponseSupported = false;
cachedWriteType = BluetoothGattCharacteristic.WRITE_TYPE_DEFAULT;
connectedDeviceId = null;
servicesDiscovered = false;
}
@Override
public void onConnectionStateChange(BluetoothGatt gatt, int newState, int status) {
if (newState == BluetoothProfile.STATE_DISCONNECTED) {
cleanupGatt();
notifyConnectionState("disconnected", null);
return;
}
// existing STATE_CONNECTED -> gatt.discoverServices();
}Why this helps
JavaScript (src/js/protocols/CapacitorBluetooth.js)
async connect(device, options = {}) {
// ... call BetaflightBluetooth.connect({ deviceId }) first
await this._waitForServices(device.deviceId, 4000); // resolves on native 'services' event
await BetaflightBluetooth.startNotifications({ deviceId: device.deviceId });
// Only now announce open
this.dispatchEvent(new CustomEvent("open", {
detail: { path: "bluetooth", baudRate: options.baudRate ?? 115200 }
}));
}
_waitForServices(deviceId, timeoutMs = 4000) {
if (this._servicesReady) return Promise.resolve();
return new Promise((resolve, reject) => {
let timeout = setTimeout(() => reject(new Error("Services discovery timeout")), timeoutMs);
const sub = BetaflightBluetooth.addListener("services", (payload) => {
if (!payload || payload.deviceId !== deviceId) return;
clearTimeout(timeout);
this._servicesReady = true;
sub.remove();
resolve();
});
});
}
async send(buffer) {
const data = buffer instanceof Uint8Array ? buffer : new Uint8Array(buffer);
try {
await BetaflightBluetooth.write({
value: arrayBufferToBase64(data),
encoding: "base64"
});
this.bytesSent += data.length;
} catch (err) {
const msg = String(err?.message || err);
if (!this._writeRetried && msg.includes("Unable to write characteristic")) {
this._writeRetried = true;
// Give GATT a breath; ensure services are ready; retry once
await new Promise(r => setTimeout(r, 60));
try {
await this._waitForServices(this.device?.device?.deviceId ?? this.device?.deviceId, 2000);
} catch (_) {}
return this.send(data);
}
this._writeRetried = false;
throw err;
}
this._writeRetried = false;
}Optional but strongly recommended (to guarantee reboot path)
Next steps to verify
If you want, I can prep a tiny follow-up patch (Java + JS) that only includes these exact changes so you can grab a CI artifact and re-test on Android 8 and 15. 🧠 Learnings used |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java (1)
1004-1032: Consider guarding against a nullcharacteristic.getService()before building the payload.The deprecated 2‑arg overload now safely guards against null
characteristic/value, which is good. In the 3‑arg overload you still assumecharacteristic.getService()is non‑null when callinggetUuid(). That’s true for normal GATT callbacks, but if a vendor stack ever delivers a characteristic without a service reference, this will crash.You could harden this cheaply:
@Override public void onCharacteristicChanged(BluetoothGatt gatt, BluetoothGattCharacteristic characteristic, byte[] value) { - if (characteristic == null || value == null) { - Log.w(TAG, "Received notification with null characteristic or value"); - return; - } + if (characteristic == null || characteristic.getService() == null || value == null) { + Log.w(TAG, "Received notification with null characteristic, service or value"); + return; + } JSObject payload = new JSObject(); payload.put("deviceId", connectedDeviceId); payload.put("service", characteristic.getService().getUuid().toString()); payload.put("characteristic", characteristic.getUuid().toString()); payload.put("value", Base64.encodeToString(value, Base64.NO_WRAP)); notifyListeners("notification", payload); }Not strictly required, but it eliminates a potential NPE edge case.
🧹 Nitpick comments (3)
src/js/msp/MSPHelper.js (1)
2935-2979: EEPROM write flow looks solid; consider small tweaks for Sonar and clarityThe new
writeConfigurationflow (quieting intervals/callbacks,sendEepromwith idempotentfinish, and the 3s fallback) is a good fit for the BLE queue/timeout issues described, and should reliably reachreinitializeConnectioneven when the MSP_EEPROM_WRITE ACK is lost. This also matches the earlier guidance to callGUI.interval_kill_all()andMSP.callbacks_cleanup()and add a conservative fallback timer. Based on learnings, this is the right direction.A few small improvements I’d suggest:
Add braces around single-line
ifstatements for Sonar + consistencySonar is flagging the inline
ifbodies; wrapping them improves readability and satisfies the rule:const sendEeprom = () => { let finished = false; const finish = () => {
if (finished) return;finished = true;
if (finished) {return;}finished = true;
if (callback) callback();
if (callback) {callback();} };
Tighten the fallback log message for non‑reboot callers (optional)
Right now the warning is hard-coded as “proceeding to reboot via fallback”, but
rebootcan be false:
const fallbackTimer = setTimeout(() => {console.warn("MSP_EEPROM_WRITE ack timeout; proceeding to reboot via fallback.");finish();}, 3000); // conservative for Android BLE
const fallbackTimer = setTimeout(() => {console.warn("MSP_EEPROM_WRITE ack timeout; finishing via fallback (reboot may still occur if requested).");finish();}, 3000); // conservative for Android BLEThis keeps the behaviour identical but avoids implying a reboot when `reboot === false`.
About the “Unexpected negated condition” warning (line 2973)
The
if (!FC.CONFIG.armingDisabled)reads naturally as “if arming is enabled”, so I’d personally leave it. If you do want to appease Sonar here, a minimal rewrite is:
- if (!FC.CONFIG.armingDisabled) {
this.setArmingEnabled(false, false);setTimeout(sendEeprom, 200); // current delay retained- } else {
setTimeout(sendEeprom, 200);- }
- if (FC.CONFIG.armingDisabled) {
setTimeout(sendEeprom, 200); // current delay retained- } else {
this.setArmingEnabled(false, false);setTimeout(sendEeprom, 200);- }
Behaviour is unchanged; it’s purely to satisfy the style rule. Overall, functionally this change looks correct and should make the BLE reboot/save path much more robust. </blockquote></details> <details> <summary>android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java (2)</summary><blockquote> `61-69`: **Align Capacitor permission alias with manual OS permission flow (or drop alias check).** Right now `@CapacitorPlugin` defines a `bluetooth` alias with no underlying strings, `requestPermissions()` does a manual `ActivityCompat.requestPermissions()` against `getRequiredPermissions()`, and `handleRequestPermissionsResult()` only resolves `pendingPermissionCall`. However, `ensureBluetoothReady()` gates all operations on `getPermissionState("bluetooth")`, and `@PermissionCallback onPermissionResult()` is never used. This means the alias is effectively decoupled from the real OS permission state and may either always pass or never transition, depending on Capacitor’s behavior for an alias with an empty `strings` array. I’d suggest either: - Wiring the alias to the real permissions via Capacitor’s permission APIs, or - Removing the `getPermissionState("bluetooth")` check and relying purely on your manual permission handling + `SecurityException` guards. In any case, please double‑check behavior on real devices (Android 8–15) so calls don’t get rejected with “Bluetooth permission not granted” despite OS permissions being granted. Also applies to: 147-196, 198-228, 230-240, 654-668 --- `572-591`: **Differentiate connection failures from normal disconnects to avoid duplicate events.** `disconnectInternal(false)` already performs cleanup and emits a `connectionState` event with reason `"disconnected"`, but `onConnectionStateChange()` calls `failConnect(...)` for *every* `STATE_DISCONNECTED`, including user‑initiated or remote disconnects after a successful session. This has two side effects: - Normal disconnects are logged as `"Connection failed: Disconnected"`. - The JS side may see two `connectionState` events for the same disconnect (one from `disconnectInternal`, one from `failConnect`). Consider splitting the logic so `failConnect()` is only used for genuine connection failures (e.g., disconnect while `ConnectionState.CONNECTING` or non‑`GATT_SUCCESS` status codes), and successful teardown after a connected session just routes through `disconnectInternal` / `notifyConnectionState` without the “failed” path. Also applies to: 611-629, 933-947 </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: Path: .coderabbit.yaml **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 6d06c2d6509a1a507f226ac5ddd3d39e415b926a and c0f286f65bf5108f0e448265d4694507de006d22. </details> <details> <summary>📒 Files selected for processing (2)</summary> * `android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java` (1 hunks) * `src/js/msp/MSPHelper.js` (1 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🧠 Learnings (21)</summary> <details> <summary>📓 Common learnings</summary>Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-01T17:09:13.412Z
Learning: In betaflight-configurator’s CapacitorBluetooth.js, native plugin listeners (notification, services, connectionState) must be attached exactly once per app lifetime. Re-attaching them during writes/connect cycles leads to duplicate handlers, increased JS workload, MSP queue pressure, and can prevent the MSP_EEPROM_WRITE callback from firing (so reinitializeConnection is not reached). Implement an idempotent _attachNativeListenersOnce() guard.Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-28T22:51:11.691Z
Learning: In betaflight-configurator, Web Bluetooth API'sservicesoption filters scan results to only devices advertising those UUIDs, whileoptionalServicesrequests permission to access services after connection without filtering. PWA (WebBluetooth.js) uses{ acceptAllDevices: true, optionalServices: uuids }to show all devices while requesting post-connection service access. CapacitorBluetooth.js should match this pattern rather than usingservicesas a scan filter.Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-02T17:46:47.828Z
Learning: BetaflightBluetoothPlugin.java: maintainers prefer write(PluginCall) to be “write-only” — no per-call UUID parsing or characteristic lookups. Resolve and cache the write characteristic once in onServicesDiscovered() (prefer PROPERTY_WRITE_NO_RESPONSE; else PROPERTY_WRITE). write() should read payload params from call, use the cached writeCharacteristic, pick write type, and submitWrite; clear the cache in cleanupGatt().Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-01T17:09:13.412Z
Learning: CapacitorBluetooth.connect should only dispatch the 'open' event after GATT services are resolved and notifications are started; announcing open earlier causes the first MSP write to fail with “Unable to write characteristic” and contributes to early queue load.Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-01T17:38:41.581Z
Learning: On BLE (Android) Save & Reboot, status pull intervals continue during MSP_EEPROM_WRITE, overwhelming the MSP queue and causing the EEPROM ack to time out so reinitializeConnection never runs. Solution: before EEPROM write, call GUI.interval_kill_all() and MSP.callbacks_cleanup(), and add a 3s fallback to proceed to reboot if the ack is missed.Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:135-143
Timestamp: 2025-11-28T22:41:59.374Z
Learning: In betaflight-configurator, WebSerial.js uses a constant path "serial" for all devices, and CapacitorBluetooth.js uses constant path "bluetooth", consistent with the single-device-at-a-time workflow. WebBluetooth.js uses counter-based paths (bluetooth_${counter}), but that's specific to its implementation. The path strategy varies by protocol based on their specific requirements.Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4706
File: src/js/protocols/CapacitorSerial.js:47-47
Timestamp: 2025-11-24T15:07:25.227Z
Learning: In betaflight-configurator's CapacitorSerial protocol (src/js/protocols/CapacitorSerial.js), the fire-and-forget this.loadDevices() call in the constructor is intentional background pre-loading. The actual critical device list loading for UI population is orchestrated by port_handler.js via serial.getDevices("serial"), which properly awaits CapacitorSerial.getDevices() → loadDevices() in a complete async chain. The constructor's async call does not cause race conditions.Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-30T18:10:26.784Z
Learning: In betaflight-configurator's Android Bluetooth implementation, permission handling must be version-aware: Android 8-11 (API 26-30) requires BLUETOOTH, BLUETOOTH_ADMIN, and ACCESS_COARSE_LOCATION permissions, while Android 12+ (API 31+) requires BLUETOOTH_SCAN and BLUETOOTH_CONNECT. The AndroidManifest.xml should use android:maxSdkVersion="30" on legacy permissions and android:minSdkVersion="31" on new permissions. The Java code must conditionally request permissions using Build.VERSION.SDK_INT >= Build.VERSION_CODES.S check to avoid requesting non-existent permissions on older Android versions.Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-06-09T00:32:21.385Z
Learning: In the betaflight-configurator codebase, port paths use counter prefixes (e.g., "bluetooth1", "bluetooth2", "serial1") rather than direct protocol identifiers. The protocol selection logic correctly usesportPath.startsWith("bluetooth")to detect bluetooth ports regardless of the counter suffix, rather than direct string matching against protocol map keys.Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4510
File: src/js/msp.js:384-391
Timestamp: 2025-09-19T20:41:44.286Z
Learning: When fixing MSP duplicate handling in Betaflight Configurator, avoid complex changes to callback resolution mechanisms as they can break tab switching functionality. Simple duplicate detection based on code and payload size is safer than complex requestKey-based approaches.Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:51:09.253Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported for BLE devices and Web USB API is supported, but Web Serial API is not supported (except limited Bluetooth serial support added in 2025). The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:39:16.388Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported from version 133+ (requires Android 6.0+) and Web USB API is supported from version 136+, but Web Serial API is not supported. The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.</details> <details> <summary>📚 Learning: 2025-12-01T17:38:41.581Z</summary>Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-01T17:38:41.581Z
Learning: On BLE (Android) Save & Reboot, status pull intervals continue during MSP_EEPROM_WRITE, overwhelming the MSP queue and causing the EEPROM ack to time out so reinitializeConnection never runs. Solution: before EEPROM write, call GUI.interval_kill_all() and MSP.callbacks_cleanup(), and add a 3s fallback to proceed to reboot if the ack is missed.**Applied to files:** - `src/js/msp/MSPHelper.js` </details> <details> <summary>📚 Learning: 2025-09-19T20:42:20.332Z</summary>Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4510
File: src/js/msp.js:384-391
Timestamp: 2025-09-19T20:42:20.332Z
Learning: Complex MSP duplicate handling fixes in Betaflight Configurator can cause infinite loading messages when changing tabs due to disruption of the callback resolution mechanism. Simple code-only duplicate detection (using this.callbacks.some((instance) => instance.code === code)) is the safer approach that preserves tab switching functionality.**Applied to files:** - `src/js/msp/MSPHelper.js` - `android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java` </details> <details> <summary>📚 Learning: 2025-09-19T20:41:44.286Z</summary>Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4510
File: src/js/msp.js:384-391
Timestamp: 2025-09-19T20:41:44.286Z
Learning: When fixing MSP duplicate handling in Betaflight Configurator, avoid complex changes to callback resolution mechanisms as they can break tab switching functionality. Simple duplicate detection based on code and payload size is safer than complex requestKey-based approaches.**Applied to files:** - `src/js/msp/MSPHelper.js` - `android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java` </details> <details> <summary>📚 Learning: 2025-08-27T19:36:13.733Z</summary>Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4579
File: src/js/tabs/pid_tuning.js:80-82
Timestamp: 2025-08-27T19:36:13.733Z
Learning: MSP_STATUS_EX calls in serial_backend.js and setup.js are intentionally unguarded because they are used for status fetching on page load to initialize profiles and are safe to call on older firmware versions. Only conditional/optional MSP_STATUS_EX usage (like in pid_tuning.js) needs version gating to API_VERSION_25_12.**Applied to files:** - `src/js/msp/MSPHelper.js` </details> <details> <summary>📚 Learning: 2025-12-01T17:09:13.412Z</summary>Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-01T17:09:13.412Z
Learning: In betaflight-configurator’s CapacitorBluetooth.js, native plugin listeners (notification, services, connectionState) must be attached exactly once per app lifetime. Re-attaching them during writes/connect cycles leads to duplicate handlers, increased JS workload, MSP queue pressure, and can prevent the MSP_EEPROM_WRITE callback from firing (so reinitializeConnection is not reached). Implement an idempotent _attachNativeListenersOnce() guard.**Applied to files:** - `src/js/msp/MSPHelper.js` - `android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java` </details> <details> <summary>📚 Learning: 2025-09-28T12:56:15.659Z</summary>Learnt from: ctzsnooze
Repo: betaflight/betaflight-configurator PR: 4627
File: src/js/tabs/onboard_logging.js:504-506
Timestamp: 2025-09-28T12:56:15.659Z
Learning: The Betaflight Configurator MSP queue can become overwhelmed during dataflash reads when retry logic is used, even with minimal retries (MAX_SIMPLE_RETRIES = 1), because failed blocks cause exponential queue growth. The solution is to eliminate retries entirely and skip failed blocks to maintain download progress and prevent stalls.**Applied to files:** - `src/js/msp/MSPHelper.js` </details> <details> <summary>📚 Learning: 2025-09-28T16:39:04.171Z</summary>Learnt from: ctzsnooze
Repo: betaflight/betaflight-configurator PR: 4627
File: src/js/FileSystem.js:101-107
Timestamp: 2025-09-28T16:39:04.171Z
Learning: In the Betaflight Configurator dataflash download implementation, removing await from FileSystem.writeChunk() and closeFile() is intentional to prevent MSP queue overwhelm. Awaiting file I/O operations slows the download loop enough to cause MSP timeouts and queue growth, leading to stalls. The non-blocking approach prioritizes download throughput over immediate file write error detection.**Applied to files:** - `src/js/msp/MSPHelper.js` </details> <details> <summary>📚 Learning: 2025-12-02T17:46:47.828Z</summary>Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-02T17:46:47.828Z
Learning: BetaflightBluetoothPlugin.java: maintainers prefer write(PluginCall) to be “write-only” — no per-call UUID parsing or characteristic lookups. Resolve and cache the write characteristic once in onServicesDiscovered() (prefer PROPERTY_WRITE_NO_RESPONSE; else PROPERTY_WRITE). write() should read payload params from call, use the cached writeCharacteristic, pick write type, and submitWrite; clear the cache in cleanupGatt().**Applied to files:** - `android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java` </details> <details> <summary>📚 Learning: 2025-11-30T18:10:26.784Z</summary>Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-30T18:10:26.784Z
Learning: In betaflight-configurator's Android Bluetooth implementation, permission handling must be version-aware: Android 8-11 (API 26-30) requires BLUETOOTH, BLUETOOTH_ADMIN, and ACCESS_COARSE_LOCATION permissions, while Android 12+ (API 31+) requires BLUETOOTH_SCAN and BLUETOOTH_CONNECT. The AndroidManifest.xml should use android:maxSdkVersion="30" on legacy permissions and android:minSdkVersion="31" on new permissions. The Java code must conditionally request permissions using Build.VERSION.SDK_INT >= Build.VERSION_CODES.S check to avoid requesting non-existent permissions on older Android versions.**Applied to files:** - `android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java` </details> <details> <summary>📚 Learning: 2025-11-28T22:51:11.691Z</summary>Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-28T22:51:11.691Z
Learning: In betaflight-configurator, Web Bluetooth API'sservicesoption filters scan results to only devices advertising those UUIDs, whileoptionalServicesrequests permission to access services after connection without filtering. PWA (WebBluetooth.js) uses{ acceptAllDevices: true, optionalServices: uuids }to show all devices while requesting post-connection service access. CapacitorBluetooth.js should match this pattern rather than usingservicesas a scan filter.**Applied to files:** - `android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java` </details> <details> <summary>📚 Learning: 2025-06-09T00:32:21.385Z</summary>Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-06-09T00:32:21.385Z
Learning: In the betaflight-configurator codebase, port paths use counter prefixes (e.g., "bluetooth1", "bluetooth2", "serial1") rather than direct protocol identifiers. The protocol selection logic correctly usesportPath.startsWith("bluetooth")to detect bluetooth ports regardless of the counter suffix, rather than direct string matching against protocol map keys.**Applied to files:** - `android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java` </details> <details> <summary>📚 Learning: 2025-05-14T21:51:09.253Z</summary>Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:51:09.253Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported for BLE devices and Web USB API is supported, but Web Serial API is not supported (except limited Bluetooth serial support added in 2025). The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.**Applied to files:** - `android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java` </details> <details> <summary>📚 Learning: 2025-11-28T22:41:59.374Z</summary>Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:135-143
Timestamp: 2025-11-28T22:41:59.374Z
Learning: In betaflight-configurator, WebSerial.js uses a constant path "serial" for all devices, and CapacitorBluetooth.js uses constant path "bluetooth", consistent with the single-device-at-a-time workflow. WebBluetooth.js uses counter-based paths (bluetooth_${counter}), but that's specific to its implementation. The path strategy varies by protocol based on their specific requirements.**Applied to files:** - `android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java` </details> <details> <summary>📚 Learning: 2025-09-09T20:02:33.475Z</summary>Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4597
File: src/js/utils/common.js:95-127
Timestamp: 2025-09-09T20:02:33.475Z
Learning: In the Betaflight Configurator codebase, the Chromium v140 compatibility issue with sortSelect was resolved by rewriting the implementation to use native DOM APIs (Array.from, select.options, select.remove, select.add) instead of jQuery DOM manipulation methods (this.children, this.empty().append). The fix still performs DOM operations but avoids the specific jQuery methods that were causing issues in Chromium v140.**Applied to files:** - `android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java` </details> <details> <summary>📚 Learning: 2025-06-27T22:06:49.210Z</summary>Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4532
File: src/js/VirtualFC.js:234-234
Timestamp: 2025-06-27T22:06:49.210Z
Learning: In the betaflight-configurator codebase, the VirtualFC.js AUX_CONFIG array must stay synchronized with the betaflight firmware's msp_box.c definitions to ensure proper auxiliary mode functionality. Changes to mode names should follow the firmware source code even if it potentially breaks backward compatibility with saved presets.**Applied to files:** - `android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java` </details> <details> <summary>📚 Learning: 2025-08-27T22:07:26.353Z</summary>Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4579
File: src/js/data_storage.js:9-9
Timestamp: 2025-08-27T22:07:26.353Z
Learning: In the betaflight-configurator CalVer refactoring, the generateVirtualApiVersions function in src/js/utils/common.js works correctly despite the apparent mathematical issue with bounds logic where compareVersions.minor() returns different ranges for API_VERSION_MAX_SUPPORTED (25.12.0) vs API_VERSION_ACCEPTED (1.44.0).**Applied to files:** - `android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java` </details> <details> <summary>📚 Learning: 2025-08-11T19:10:56.992Z</summary>Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4567
File: locales/en/messages.json:7724-7728
Timestamp: 2025-08-11T19:10:56.992Z
Learning: In the Betaflight Configurator project, only English language entries are added directly to locales/en/messages.json. Translations for other languages are handled through an outsourced process and should not be suggested as part of code reviews.**Applied to files:** - `android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java` </details> <details> <summary>📚 Learning: 2025-09-10T18:26:10.136Z</summary>Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4601
File: package.json:125-125
Timestamp: 2025-09-10T18:26:10.136Z
Learning: In betaflight-configurator, dependency updates are handled incrementally - Vue dependencies are updated separately from Vite dependencies for better isolation and maintainability.**Applied to files:** - `android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java` </details> <details> <summary>📚 Learning: 2025-11-19T17:55:51.714Z</summary>Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4708
File: src/js/tabs/firmware_flasher.js:0-0
Timestamp: 2025-11-19T17:55:51.714Z
Learning: In the betaflight-configurator project, only current Chrome and Edge-based browsers are officially supported, and users are expected to keep their browsers up-to-date for security reasons. This means ES2023 features like Array.prototype.toSorted() and Object.groupBy() can be safely used without polyfills or transpilation.**Applied to files:** - `android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java` </details> <details> <summary>📚 Learning: 2025-12-01T17:09:13.412Z</summary>Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-01T17:09:13.412Z
Learning: CapacitorBluetooth.connect should only dispatch the 'open' event after GATT services are resolved and notifications are started; announcing open earlier causes the first MSP write to fail with “Unable to write characteristic” and contributes to early queue load.**Applied to files:** - `android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java` </details> </details><details> <summary>🧬 Code graph analysis (1)</summary> <details> <summary>src/js/msp/MSPHelper.js (5)</summary><blockquote> <details> <summary>src/js/gui.js (1)</summary> * `GUI` (473-473) </details> <details> <summary>src/js/msp.js (1)</summary> * `MSP` (5-455) </details> <details> <summary>src/js/serial_backend.js (1)</summary> * `reinitializeConnection` (798-834) </details> <details> <summary>src/js/receiver_msp/receiver_msp.js (1)</summary> * `i18n` (8-8) </details> <details> <summary>src/js/localization.js (1)</summary> * `i18n` (7-7) </details> </blockquote></details> </details><details> <summary>🪛 GitHub Check: SonarCloud Code Analysis</summary> <details> <summary>src/js/msp/MSPHelper.js</summary> [failure] 2947-2947: Expected { after 'if' condition. See more on https://sonarcloud.io/project/issues?id=betaflight_betaflight-configurator&issues=AZrgvBneYqGoADY79vbO&open=AZrgvBneYqGoADY79vbO&pullRequest=4714 --- [failure] 2955-2955: Expected { after 'if' condition. See more on https://sonarcloud.io/project/issues?id=betaflight_betaflight-configurator&issues=AZrgvBneYqGoADY79vbP&open=AZrgvBneYqGoADY79vbP&pullRequest=4714 --- [warning] 2973-2973: Unexpected negated condition. See more on https://sonarcloud.io/project/issues?id=betaflight_betaflight-configurator&issues=AZrgvBneYqGoADY79vbQ&open=AZrgvBneYqGoADY79vbQ&pullRequest=4714 </details> </details> </details> <details> <summary>⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)</summary> * GitHub Check: deploy </details> <details> <summary>🔇 Additional comments (1)</summary><blockquote> <details> <summary>android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java (1)</summary><blockquote> `114-116`: **Notification queuing and service-discovery sequencing look solid.** Using a `ConcurrentLinkedQueue` for `pendingStartNotificationCalls`, queuing `startNotifications` while `servicesDiscovered == false`, and flushing them from `onServicesDiscovered()` gives a clean, race‑free path. The reject path also correctly cancels outstanding calls on connect failures/disconnects. This addresses the earlier race around starting notifications before services are ready in a robust way. Also applies to: 357-369, 841-867, 948-1002 </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
...app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
Show resolved
Hide resolved
|
|
Preview URL: https://pr4714.betaflight-app-preview.pages.dev |
2 participants
issues remaining
Summary by CodeRabbit
New Features
Chores / Improvements
✏️ Tip: You can customize this high-level summary in your review settings.